3 ms·
Configuration variables make the code ever-so-slightly longer, and introduces additional failure points (what if the config file is missing or corrupted?) and c
by yongjik 10y ago
Configuration variables make the code ever-so-slightly longer, and introduces additional failure points (what if the config file is missing or corrupted?) and considerations for cases that will never arise with hard-coded values (should we check that the limit is always positive? what if it is 10000 months, should we do something?).
Yes, in many cases they are an improvement, but not always. Every "best practice" must be judged individually with the question "does it apply here?".
Years ago I was working with some very small piece of a web-crawling framework, and I had to do something special for www.naver.com, Korea's biggest portal. Because the string "www.naver.com" appeared multiple times in the same function, we put that into a string constant at the beginning: something like
static const string kNaverUrl = "www.naver.com";
Because having "magic constant" sprinkled in the code is bad!
...it was only much later that I realized the stupidity. As if Naver, Korea's #1 portal, will change its domain name at a whim! Even in the unlikely case that happens (say, they get acquired), so many other things will also change that my code will be surely unusable after that.
kNaverUrl was an unnecessary detour that only served to make the code longer and (slightly) less obvious, guarding against a theoretical possibility that will never come true.
Code Review has this nasty tendency of accumulating "best practices" beyond breaking point, because nobody objects to best practices. Once in a while, these practices turn out not to be that best.
Just my two cents.
- serge2k 10y agoIt's also easier to refactor code without magic constants, as you have compile time bugs mispelling kNaverUrl instead of runtime bugs because "ww.naver.com" in one of 10 places.
- yongjik 10y agoYou have a point, but on the other hand, it is much easier to write (kNaverUrl + "index.html") than to write "www.naver.comindex.html". So it just opens door for a different kind of bugs.
- zepolen 10y agoThen it becomes a function: kNaverUrl('index.html') Multiple functions: kNaverWwwUrl('index.html') kNaverApiUrl('resource') Then it get's refactored: urlGenerator('www.naver.com')('index.html') urlGenerator('api.naver.com')('resource') Refactored even more: urlFor('naver_www', 'index.html') urlFor('naver_api', 'resource') Even more! urlGeneratorFactoryFactory(.......
- dozzie 10y agoI don't know how does your editor work, but my Vim diligently helps me with identifier completion, so kNaverUrl wins every time with "www.naver.com", even if it doesn't make the code more flexible. It also defends against typos, as serge2k pointed.
- jeroen 10y ago> I had to do something special for www.naver.com, Korea's biggest portal. Because the string > "www.naver.com" appeared multiple times in the same function, we put that into a string > constant at the beginning: something like > static const string kNaverUrl = "www.naver.com"; > Because having "magic constant" sprinkled in the code is bad! You have changed your magic string into a magic constant. It's a step towards better code, but the next step is to give the thing a proper name. In this case, something like DoSomethingSpecialUrl. Now you have a bit of self-documenting code.
- zelos 10y agostatic const string kNaverUrl = "www.naver.com"; That's such a common junior developer mistake. I see it a lot - constants called ONE_SECOND or COMMA.
- hackflip 10y ago> As if Naver, Korea's #1 portal, will change its domain name at a whim Maybe they won't change their domain name, but what if you end up needing to move from http to https? Or what if you want to crawl their staging server at staging.naver.com? Or if you wanted to bypass the DNS? Avoiding the magic constant achieves more than just guarding against name changes. It also guards against typos, you reduce n potential failure points to 1.