3 ms·

I can relate. At my first internship, there was a code quality tool forced on the team by management as well. It had a rule to "disable magic numbers". The res

I can relate. At my first internship, there was a code quality tool forced on the team by management as well. It had a rule to "disable magic numbers". The result was a header with: static const unsigned ONE = 1; static const unsigned TWO = 2; static const unsigned THREE = 3; ... Up to some thousands.

isoprophlex 3y ago
Heavens forbid you want to raise some number to the power of 2, err I mean TWO
awestroke 3y ago
Really? You could not come up with a way to avoid magic numbers?
- Retric 3y ago
  Many uses of numbers aren’t magic numbers. If you want to count something then setting the counter to 0 and incrementing by 1 isn’t unexplained or subject to change.
- GuB-42 3y ago
  Sometimes, a number is just a number. The average of a, b, c is (a+b+c)/3, an positive interger is single digit when less then 10, the formula for the volume of a sphere is 4/3PIr^3, etc... And that's excluding 0, 1 and 2 which are naturally everywhere. That some numbers are not magic would be obvious to a human reviewer, but the tool probably just treats any number in an expression as a magic number or something like that, and the workaround is to define constants for raw numbers. Which entirely defeats the purpose since now, people will just use these constants for actual magic numbers and the tool will see nothing.
  - jader201 3y ago
    Maybe in rare cases, but in my experience, literals (numbers or strings) almost always have _some_ meaning where giving them a name helps readability. In my experience, it’s rare that “a number is just a number”. And sure, in those cases, naming them is silly.
  - aqme28 3y ago
    Listen, what happens if we want to calculate delivery distances in a n-dimensional universe?
  - 542458 3y ago
    That 3 in your first example is definitely a magic number that should be either dynamically calculated from the number of elements being averaged, or defined as a const NUMBER_ELEMENTS_AVERAGED.
    - polynomial 3y ago
      Sometimes a magic number is just a magic number.
    - barco 3y ago
      If you name it `NUMBER_ELEMENTS_AVERAGED`, then when you add a new element to average, you will miss the fact that you also need to modify that value :) You either have them on a list and calculate it dynamically based on the size, or have it as a magic number.
    - GuB-42 3y ago
      In the general case, of course you would use arrays (static or dynamic) and some kind of "size" attribute. But this is just 3 values in an expression and using a constant could actually be bad. Let's be a bit more practical. int lightness(int r, int g, int b) { return (r+g+b)/3; } Simple and straightforward int lightness(int r, int g, int b) { const int NUMBER_ELEMENTS_AVERAGED = 3; return (r+g+b)/NUMBER_ELEMENTS_AVERAGED; } Ok, I guess, but I think verbose for no good reason. But not as bad as the seemingly "cleaner" const int NUMBER_OF_COLOR_COMPONENTS = 3; int lightness(int r, int g, int b) { return (r+g+b)/NUMBER_OF_COLOR_COMPONENTS; } Imagine that you want want to add a color component, for example to support transparency (alpha). So you set NUMBER_OF_COLOR_COMPONENTS = 4, and then, your "lightness" function breaks, the simple (r+g+b)/3 would have stayed correct. That happened because didn't get the real meaning of that "3". Even if semantically, at the time you written that code, it is the number of color components, in reality, it is the number of terms in the expression. There is r, g, b: 3 terms, so 3. Who cares how many color components there are? Side note: I know it is the wrong formula for lightness, that's just an example.
- drowsspa 3y ago
  Sometimes it's much clearer the number itself, specially when it's a mathematical formula. For instance, is const MILLIS_PER_SECOND = 1000; ... ... ... ... ... const durationMs = MILLIS_PER_SECOND * duration really clearer than const durationMs = 1000 * duration ?
  - RHSeeger 3y ago
    Along the same lines, I see this a lot long delay = (5 * 60 * 1000); // 5 minutes, in millseconds And it's perfectly clear to me. Now, I think the comment is really helpful there (indicating intent), but I don't think having separate constants for each of the numbers there is going to make the code better. As it is, it's very easy to read at a glance, know what it's intended to do, and determine if it's correct (should you be worried about that at the moment). Which is what's important there.
    - gregmac 3y ago
      I do that as well, but typically name that variable delayMilliseconds so there's no confusion. sleep(delay) always looks ok, but sleep(delayHours) is probably going to catch your eye as suspicious.
  - squeaky-clean 3y ago
    The latter one leaves the possibility for this though const duration = 1000 * duration
  - thebackup 3y ago
    Capital M is for Mega. I would use duration_s and duration_ms. const duration_ms = 1000 * duration_s And _us for microsec. const duration_us = 1000 * duration_ms But then the tool would probably reject my code for not following the naming conventions which ”disallows using underscores in variable names”. Guess what I wanted to say is that there are always exceptions to the rule and there should always be some way to turn off the automatic checker for certain sections of the code.
- strulovich 3y ago
  I believe even truly magic numbers don’t need to be extracted in many cases. Say in some UI code called FooBarItem you suddenly have some call to set padding to 12. People will take this number, put it in constant and name it FOO_BAR_ITEM_PADDING = 12. This is not better. It’s just jumping more through the code, and whatever is in the name of the constant is easily deductible from the usage pattern. I learnt that pattern from others and nowadays I see it as useless. If you can add interesting info in the name or comments of the number, don’t bother extracting a constant.
  - lucianbr 3y ago
    Don't get me started on string constants. OK_LABEL_TEXT = "OK". GET_METHOD = "GET". Bonus points if this is only used in exactly one place. Is it really difficult to see that these are pointless? If you actually think about what you are writing, that is.

isoprophlex 3y ago

Heavens forbid you want to raise some number to the power of 2, err I mean TWO

awestroke 3y ago

Really? You could not come up with a way to avoid magic numbers?

Retric 3y ago
Many uses of numbers aren’t magic numbers. If you want to count something then setting the counter to 0 and incrementing by 1 isn’t unexplained or subject to change.
GuB-42 3y ago
Sometimes, a number is just a number. The average of a, b, c is (a+b+c)/3, an positive interger is single digit when less then 10, the formula for the volume of a sphere is 4/3PIr^3, etc... And that's excluding 0, 1 and 2 which are naturally everywhere. That some numbers are not magic would be obvious to a human reviewer, but the tool probably just treats any number in an expression as a magic number or something like that, and the workaround is to define constants for raw numbers. Which entirely defeats the purpose since now, people will just use these constants for actual magic numbers and the tool will see nothing.
- jader201 3y ago
  Maybe in rare cases, but in my experience, literals (numbers or strings) almost always have _some_ meaning where giving them a name helps readability. In my experience, it’s rare that “a number is just a number”. And sure, in those cases, naming them is silly.
- aqme28 3y ago
  Listen, what happens if we want to calculate delivery distances in a n-dimensional universe?
- 542458 3y ago
  That 3 in your first example is definitely a magic number that should be either dynamically calculated from the number of elements being averaged, or defined as a const NUMBER_ELEMENTS_AVERAGED.
  - polynomial 3y ago
    Sometimes a magic number is just a magic number.
  - barco 3y ago
    If you name it `NUMBER_ELEMENTS_AVERAGED`, then when you add a new element to average, you will miss the fact that you also need to modify that value :) You either have them on a list and calculate it dynamically based on the size, or have it as a magic number.
  - GuB-42 3y ago
    In the general case, of course you would use arrays (static or dynamic) and some kind of "size" attribute. But this is just 3 values in an expression and using a constant could actually be bad. Let's be a bit more practical. int lightness(int r, int g, int b) { return (r+g+b)/3; } Simple and straightforward int lightness(int r, int g, int b) { const int NUMBER_ELEMENTS_AVERAGED = 3; return (r+g+b)/NUMBER_ELEMENTS_AVERAGED; } Ok, I guess, but I think verbose for no good reason. But not as bad as the seemingly "cleaner" const int NUMBER_OF_COLOR_COMPONENTS = 3; int lightness(int r, int g, int b) { return (r+g+b)/NUMBER_OF_COLOR_COMPONENTS; } Imagine that you want want to add a color component, for example to support transparency (alpha). So you set NUMBER_OF_COLOR_COMPONENTS = 4, and then, your "lightness" function breaks, the simple (r+g+b)/3 would have stayed correct. That happened because didn't get the real meaning of that "3". Even if semantically, at the time you written that code, it is the number of color components, in reality, it is the number of terms in the expression. There is r, g, b: 3 terms, so 3. Who cares how many color components there are? Side note: I know it is the wrong formula for lightness, that's just an example.
drowsspa 3y ago
Sometimes it's much clearer the number itself, specially when it's a mathematical formula. For instance, is const MILLIS_PER_SECOND = 1000; ... ... ... ... ... const durationMs = MILLIS_PER_SECOND * duration really clearer than const durationMs = 1000 * duration ?
- RHSeeger 3y ago
  Along the same lines, I see this a lot long delay = (5 * 60 * 1000); // 5 minutes, in millseconds And it's perfectly clear to me. Now, I think the comment is really helpful there (indicating intent), but I don't think having separate constants for each of the numbers there is going to make the code better. As it is, it's very easy to read at a glance, know what it's intended to do, and determine if it's correct (should you be worried about that at the moment). Which is what's important there.
  - gregmac 3y ago
    I do that as well, but typically name that variable delayMilliseconds so there's no confusion. sleep(delay) always looks ok, but sleep(delayHours) is probably going to catch your eye as suspicious.
- squeaky-clean 3y ago
  The latter one leaves the possibility for this though const duration = 1000 * duration
- thebackup 3y ago
  Capital M is for Mega. I would use duration_s and duration_ms. const duration_ms = 1000 * duration_s And _us for microsec. const duration_us = 1000 * duration_ms But then the tool would probably reject my code for not following the naming conventions which ”disallows using underscores in variable names”. Guess what I wanted to say is that there are always exceptions to the rule and there should always be some way to turn off the automatic checker for certain sections of the code.
strulovich 3y ago
I believe even truly magic numbers don’t need to be extracted in many cases. Say in some UI code called FooBarItem you suddenly have some call to set padding to 12. People will take this number, put it in constant and name it FOO_BAR_ITEM_PADDING = 12. This is not better. It’s just jumping more through the code, and whatever is in the name of the constant is easily deductible from the usage pattern. I learnt that pattern from others and nowadays I see it as useless. If you can add interesting info in the name or comments of the number, don’t bother extracting a constant.
- lucianbr 3y ago
  Don't get me started on string constants. OK_LABEL_TEXT = "OK". GET_METHOD = "GET". Bonus points if this is only used in exactly one place. Is it really difficult to see that these are pointless? If you actually think about what you are writing, that is.