The Programming Metagame

There are lots of things to check when doing a code review, the first thing you must look for is if the code you are looking at is doing what it is supposed to, you also need to check that it's not doing dangerous things like leaving memory unallocated or mutating the parent state.

But the thing I have been looking after the most lately is how the code expresses what is doing, let me explain.

When you write code, two things are happening at different levels, the most basic level is what the code is doing, the actual algorithm that it represents; the most common example would be sorting numbers, where you get a list of numbers and return the list ordered.

Code you end up writting or reading everyday is not common though, and that means you need to extract from the code, what the intentions of the original author really are, for example some javascript:

function calculate(a, b) {
    //...
}

Although the code might return the correct result, I consider it bad coding since the neither name of the function, nor the name of the variables express any idea of what is supposed to happen inside. A good function name gives you a clear idea of what it's supposed to do, same for the parameters it accepts. Are a and b supposed to be numbers?, strings? or perhaps objects?, good naming can help.

function calculate3DIntersectionPoint(triangle, ray) {
    //...
}

I now it's a big name for a function, but that makes it expressive and gives you a much better idea of whats happening in there: it's going to calculate the intesection point in three dimensional space for two objects, a triangle and a ray.

function calculate3DIntersectionPoint(triangle, ray) {
    const origin = ray.origin ? ray.origin : [0,0,0];
    ...
    return ray.origin.map(x => x + distance);
}

Now as for what's happening inside, is ray.origin supposed to be null or not? The confusion comes by looking at the first and last line. The first one is saying: ray.origin can be null while the last line is expressing the opposite of that, which is the correct assumption?

The author might say that ray.origin can never be null, then the correct way to write this function is not to do the null check in the first line:

function calculate3DIntersectionPoint(triangle, ray) {
    const origin = ray.origin;
    ...
    return ray.origin.map(x => x + distance);
}

Now you now ray.origin is not going to be null when the function is called. The other case might happen as well:

function calculate3DIntersectionPoint(triangle, ray) {
    const origin = ray.origin ? ray.origin : [0,0,0];
    ...
    return ray.origin ? ray.origin.map(x => x + distance) : [distance, distance , distance];
}

Could this been written better? yes --like checking first thing and returning an error, for example--, but I wanted to stress the point that, if you are checking for an assumption over some data in one part of your code, you need to keep checking for it in all other places you are using int.

And this doesn't just applies to code, but to project structures as well, giving importance to what file names are used, how are stored or how parts of the project communicate with each other.

Help your team understand what you mean, and help your future self in the process.


Enrique CR
All posts