5
\$\begingroup\$

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
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 15, 2015 at 19:16
\$\endgroup\$
4
  • 5
    \$\begingroup\$ Don't use such variable names (t, j) use descriptive names like tree, root, node ... and t->child is more readable than (*t).child \$\endgroup\$ Commented Oct 15, 2015 at 18:43
  • \$\begingroup\$ Leading _ is a problem. May conflict with future keywords: see stackoverflow.com/questions/26217029/macro-naming-convention/… \$\endgroup\$ Commented Oct 15, 2015 at 19:18
  • 2
    \$\begingroup\$ I have rolled back two of your edits. Please see What to do when someone answers . \$\endgroup\$ Commented Oct 15, 2015 at 19:58
  • \$\begingroup\$ BTW the comments are awful. They look like a mangled translation of your C code to English. Don't do that! Make your comments more high-level, e.g. your comment "Adds child to the given treeNode" is perfectly good. If someone wants to know how your code does that, he can run it in a debugger (or trace the code's execution in one's mind). Also, the comment on 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\$ Commented Oct 15, 2015 at 20:07

3 Answers 3

2
\$\begingroup\$

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;
answered Oct 15, 2015 at 19:38
\$\endgroup\$
0
4
\$\begingroup\$

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.

answered Oct 15, 2015 at 18:43
\$\endgroup\$
18
  • 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\$ Commented 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\$ Commented 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\$ Commented 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\$ Commented 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\$ Commented Oct 15, 2015 at 21:42
0
\$\begingroup\$

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
}
answered Oct 16, 2015 at 7:51
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.