Looking at my code, I feel like it is not instantly clear for someone who did not write it. I feel like readability will be important for me in the future, if I'm ever to write code on an enterprise, or open source project. Do you have any advice as to how I can make this and future C more readable?
#include "datatypes.h"
#include <stddef.h>
//Changes the node's child (if it doesn't exist) to newChild
//or appends newChild to the end of the node's child's sibling list
void addChild ( treeNode *root, treeNode *newChild ) {
treeNode *temp;
if ( root->child == NULL ) {
root->child = newChild;
} else {
temp = root->child;
while ( temp->sibling != NULL ) {
temp = temp->sibling;
}
temp->sibling = newChild;
}
}
//Returns a new treeNode with default data values or {0,NULL,NULL}
treeNode newNode () {
treeNode node = {0, NULL, NULL};
return node;
}
datatypes.h
typedef struct treeNode {
int n;
struct treeNode *child;
struct treeNode *sibling;
} treeNode;
treeNode newNode (); //Returns a treenode with default {0,NULL,NULL}
void addChild ( treeNode *t, treeNode *c ); //Adds child to the given treeNode
3 Answers 3
The code is pretty clear for anybody with C
background. One suggestion though - there is no need for temp
; you may safely reuse node
:
node = node->child;
while (node->sibling) {
node = node->sibling;
}
node->sibling = child;
Comment, Comment, Comment. If you want any other person on this blue planet to know whats going on in your code in the easiest way possible, comment your code.
// This is what the code does in the next line.
/* I'm going to write a block of text to tell you the purpose of what this function
is trying to accomplish */
Your code doesn't have to be pretty or legible for that matter, but a few comments go along way.
Also, whitespace for your if
and while
statements are appealing to the eye and allow the reader to break apart your conditional statements. Trying to cram as much as you can on one line causes headaches when trying to interpret them.
-
4\$\begingroup\$ I disagree. Comments are often more a code smell than a helpfull hint. Also, they tend to get wrong over time. Try to refactor the code so that your intent becomes clear (good variable names, good function names, easy-to-read-coding style, short functions, etc.). \$\endgroup\$Peter Schneider– Peter Schneider2015年10月15日 18:56:22 +00:00Commented Oct 15, 2015 at 18:56
-
4\$\begingroup\$ @PeterSchneider: Real coders do not comment, huh? Sorry, but this is... well... just wrong. Code only tells you "how". Comments should of course not duplicate that, they should tell you "what" (one level up). And then the documentation should tell you the "why". You can write the clearest code in the world, that does not make comments unnecessary. At the very least, they can provide an indication for the poor sod debugging your code where your "how" didn't actually do "what" you intended... \$\endgroup\$DevSolar– DevSolar2015年10月15日 19:04:47 +00:00Commented Oct 15, 2015 at 19:04
-
1\$\begingroup\$ I've been working on two commercial products with a10 years plus history. Sometimes it's a funny game to trace a comment back in time if it made sense back then and when they become missleading. I know this isn't optimal but people tend to ignore documentation and dive directly into the code fixing the current problem and do not update the comment. Maybe I had bad luck, maybe its typical. Don't get me wrong good comments are great - but bad ones are worse than none. And I comment my code if it is necessary BTW ;) (math stuff, why this nasty workaround and not the obvious way, etc.) \$\endgroup\$Peter Schneider– Peter Schneider2015年10月15日 20:55:21 +00:00Commented Oct 15, 2015 at 20:55
-
1\$\begingroup\$ @KillaBytes: "the product" is the final binary with all of it's new features and bugfixes compared to the last version. Comments are internal documentation and "the company" doesn't get money for it. Managers are pushing for code - not for comments ;) As a professional you can (and should!) try to improve the internal code quality. But management will almost never ask for this and it will only work in small steps and it is a long march. \$\endgroup\$Peter Schneider– Peter Schneider2015年10月15日 21:18:11 +00:00Commented Oct 15, 2015 at 21:18
-
2\$\begingroup\$ @KillaBytes: officially, I only do what my manager is asking for. Inofficially I try to improve the code base with every commit (this rarely means comments for me but sometimes they are an improvement). The trick is to find a way between. You do not get paid for 8 hours maintenance a day. If you do zero, your code base is rotten in 2 years. \$\endgroup\$Peter Schneider– Peter Schneider2015年10月15日 21:42:29 +00:00Commented Oct 15, 2015 at 21:42
You can easily simplify your addChild
-method:
void addChild ( treeNode *root, treeNode *newChild ) {
treeNode** p = &root->child;
while(*p)
p = &p[0]->sibling;
*p = newChild;
}
And there's no reason for newNode
to exist as it's trivial (C99 being available for compound-literals):
treeNode newNode () {
return (treeNode){0}; // C needs at least one initializer
}
t
,j
) use descriptive names liketree
,root
,node
... andt->child
is more readable than(*t).child
\$\endgroup\$_
is a problem. May conflict with future keywords: see stackoverflow.com/questions/26217029/macro-naming-convention/… \$\endgroup\$newNode
is similarly long-winded, useless and error-prone - remove it or replace it with something like "Creates an empty tree". Also, the comment "and returns the root node" is incorrect (obsolete?) - remove it. \$\endgroup\$