Improve your code: Take care of "magic values" in code
July 8, 2008 5:20 pm Improve Your CodeWe all know that “magic values” can be bad but we still use them. Sometimes we need them to represent specific states like not initialised, or almost initialised or some other type of placeholder. The simple fact that we still see magic values in code in every project it means developers still use them.
Anyway, one day we had a problem on one of our views and we traced the problem to some wrong checks for the magic value of a specific id. We fixed the problem and also changed the “magic” value from the “magic 0 (zero)” to the less magic and obvious “-1”. We thought we fixed the problem until we discovered that the code had some tests not using the constant but checking directly against zero.
What we learned:
- If you need to use a magic value to represent a specific “non-existent” information NEVER user 0 (zero) or default(T). Zero is the default for an integer so your might hit your “magic” value even in the scenarios that you don’t expect.
- Never assume that if you could parse a string into an integer the value you just parsed is a valid value. We parsed a zero and the code simply assumed that if the parse worked the value is good. The parse worked with zero and the code did a (zero – 1) then referenced an array. It failed. Always validate the results of the parse.
- If you need to check against a magic value ALWAYS check against the constant and NEVER against the magic value itself.
- Obviously, this code:
private const int Constants.ValueNotInitialised = 0; _someId = Constants.ValueNotInitialised; if (_someId == 0) { ... }will fail when we modified Constants.ValueNotInitialised to be -1 and not zero.
A nice way to make sure none of your (non-string) magic values are safe and never used the wrong way is to declare your (non-string) constants using a non-fixed value:
#if DEBUG private readonly int MyMagicConstant = -Environment.TickCount; #else private const int MyMagicConstant = -1; #endif
On every debug run you’ll get a new MyMagicConstant value.