Friday, February 4, 2011

Reviewing patches backwards

Though this may be obvious to some, I only learned a short while ago that it's much easier to review patches bottom to top. Why? Because much code is written bottom to top.

In C, one uses static functions for code that doesn't need to be exported. Because we're lazy and try to group code together, many statics don't have a separate declaration. So you end up writing with code like this:


static void cure_world_peace(void) {
...
}

static void save_cancer(void) {
...
}

static void solve_all_problems(void) {
cure_world_peace();
save_cancer();
}

void foo(void) {
...
solve_all_problems();
...
}


When you're reviewing top to bottom, you see the new feature first but lack the context. This gets more and more confusing the more functions you see because you have to keep in mind what they're doing. Reviewing bottom to top means you'll see the context first and you can tick it off mentally. Only then will get to the static functions and see the details.

And coincidentally, this is also how I write code - smack the hooks where they belong and then implement the details.

1 comment:

Anonymous said...

This is always something I have to remind myself when I'm reading someone else's code. Every other time I look at new code, I accidentally look at the top of the file by accident and have to correct myself.