You’ve probably heard that a developer of an established software project writes an average of 100 lines of code (loc) a day. I can say that this applies to me. So if you write 100 loc per day, how many do you read? I would estimate that the amount of time you spend on reading and understanding code is significantly more than on writing code. You probably also spend quite some time debugging code.
If you spend so much more time on reading and debugging code than writing code, shouldn’t you put more effort in writing clean and debuggable code? The Samba codebase is pretty old, more than 15 years now. I would say we have some experience with bad code and we have started to write much better and cleaner code, because we have wasted so much time trying to understand and debug code. However there is still room for improvement. Lets take a look at the following C code snippet.
if (!a) {
return;
}
if (!b) {
return;
}
if (!c) {
return;
}
if (!*d) {
return;
}
Can you guess from the code above what types of variables a, b, c and d are? The answer is no? Ok, lets take a look at the following code:
if (a == NULL) {
return;
}
if (b == 0) {
return;
}
if (c == false) {
return;
}
if (d[0] == '\0') {
return;
}
If you look at the code now, you can probably guess what types they are. Well not exactly which type, but in which superset they are. ‘a’ is a pointer, ‘b’ is an integer type, ‘c’ is a bool and ‘d’ is a string (char array). If you write code the way shown above, you don’t have to scroll up to find out as which type the variable is defined. Most of the time it is enough to know what type of superset you are checking and why.
Think about this: If you spend just a bit more time on writing clean code now, you will spend less time on reading, understanding and debugging the code later if you have to find a bug.
Lets look at some more best practices we do in the Samba code:
bool ok;
int rc;
/*
* bool return codes should always have the name 'ok' or
* start with 'is_' or 'do_'
*/
ok = fn_returning_a_bool();
if (!ok) {
return;
}
/* We use rc or ret for an integer return code */
rc = do_something();
if (rc < 0) {
return;
}
You can see that we have variables for the return codes and check them with an if-clause. The reason is that it is easy on the eyes and in a debugger you can simply print the return code variable. If you write it like this: if (do_something() < 0) You have a hard time in the debugger to find out the actual return code. You have to step into and through the function to get it. We allow the !ok syntax for bool types, cause ok is by definition in our code a bool.
To be continued ...
