loading...

local constants vs comments

peledzohar profile image Zohar Peled ・1 min read

Consider the following code (c#, but I hope readable for developers of other languages also):

bool IsCorrectType(int inputType)
{
    // 2 is the only correct type.
    return inputType == 2; 
}

against the following code:

bool IsCorrectType(int inputType)
{
    const int correctType = 2; 
    return inputType == correctType;
}

Which would you consider more readable, and why?

(of course, this is only a simplified example, actual code is more complicated)

For non c# readers:
// is be beginning of a line comment,
and the const int in the second code snippet is a local constant - the compiler replaces each occurrence of it with it's value - so performance wise these codes are equal.

Discussion

pic
Editor guide
 

Coming from C/C++, I'd go for option 2 but in all caps to show at a glance that it's a hardcoded value. Not sure if that's also a convention in C#.

const int CORRECT_TYPE = 2;
return inputType == CORRECT_TYPE;

 

I'l all for all cups for constants, however standards where I work demand pascal casing for constants (for some made-up b.s reason), so I got used to it...

 

The second option is objectively superior. It's not just a readability thing, though I would argue it's more fluid/natural and thus more readable.

There's a reason almost every linter will flag "magic numbers" like the '2' in your first example. Not only does labeling these with names make the code read more clearly, it's less error-prone. What happens when the magic value changes from 2 to 3? Somebody changes the value, but forgets to update the comment, because comments are often mentally invisible when working with code. Maybe code review catches it, maybe it doesn't. Also, almost any magic number like this will appear in many places, so the local const should and will be promoted to a higher scope; the second style prepares for that with code.

Lastly, the second version is self-documenting, with all the benefits that come with that (which have been discussed ad nauseum elsewhere, so I won't).

 

Good reasoning!

 

I'd go with the second approach, but when needed I'd add a comment explaining concretely why that value is the only correct value.

 

Well, this is just a silly example, real code would have things like if(event.code == 1903) // own goal vs
const int ownGoal = 1903;...if(event.code == ownGoal) - so no, you don't really need the comment. I also tend to pick the second option, but I can't really explain why - it just feels like a better option...