4 ms·

I'm going to assume you're not deliberately trying to misunderstand him for the sake of a hot take, because, that would be pretty lame. So instead I'll explain

by ztjio 6y ago

I'm going to assume you're not deliberately trying to misunderstand him for the sake of a hot take, because, that would be pretty lame. So instead I'll explain for you and anyone else who doesn't understand him what he meant. What he is suggesting is that you generally not include literals in your code, and instead, use a constant/variable that can be traced back to a single place in order to make changes more visible and easier to deal with. That he makes exceptions for 1, 0, and -1 is explained by conventions in various languages where those values in context end up as generic and meaningful as any other language keyword and thus would not benefit from being referenced from a constant. That doesn't mean you wouldn't ever end up with constants that are 0, 1 or -1 though, just that you wouldn't assume every random place you might use those values (such as sorting algorithms) justifies a constant placeholder. For further review on the topic, may as well start here: https://en.wikipedia.org/wiki/Magic_number_%28programming%29 https://en.wikipedia.org/wiki/Magic_number_%28programming%29

klyrs 6y ago
> I'm going to assume you're not deliberately trying to misunderstand him for the sake of a hot take, because, that would be pretty lame. I'll admit, my example was overwrought with comedic intent, but I stand firm on this issue. Use a magic number twice? Yeah, go ahead and put that numeric literal somewhere convenient (but you still have a numeric literal) to the consumers of it and easy to find by your readers (not a top-level magic_numbers.h) I'm a mathematician. The fear of numbers in code is an affront to the domain that I work in. YMMV. I put a lot of work into documenting my code, but for the love of pete, 2 is not a magic number: my example was no more overwrought than OP's rule.
- mattkrause 6y ago
  It depends on why you're using the 2. In some cases, I'd say that 1 can be a magic number. For example, it would better to write fprintf(STDOUT, output_string) than fprintf(1, output_string) However, 0 and ±1 have a special role in picking out the first/next/previous elements of a sequence (and related things), which is so common that it'd be silly to insist on defining POSSIBLE_NEXT_INDEX and POSSIBLE_PREDECESSOR. That said, people do seem to love Python's itertools, so....maybe that's where we're headed. I had somebody aggressively complain about the readability of the all-pairs for-loop, which I thought was basically standard. for(auto i=0; i<N-1; i++) for(auto j=i+1; j<N; j++) process(X[i], X[j]) The flip side is that I don't think you always need to do this. For example, this is silly: double triangle::area() const { const double NORMALIZATION = 1/2.0; return this->base * this->height * NORMALIZATION; } The point is just to make it clear why that particular number is being used and where it came from.
  - rmtech 6y ago
    The all pairs loop is unnecessarily low level, and it would be hard to spot a mistake there. It would be great if there was a way to say for i < j <= N as this is super clear.
    - mattkrause 6y ago
      I guess so. I don't think it's atrocious: it looks like iterating over the upper/lower triangular part of a matrix to me, which is fairly common (in some domains, I guess). Plus, I like that the double for-loop indicates slowness. However, Python has itertools.combinations(X,2) and Julia has IterTools.subsets(X,2) if that's what you want.
      - klyrs 6y ago
        I think you mean # see __magic_numbers.py itertools.combinations(X, number_of_things_in_a_pair)
        deleted 6y ago
        [deleted]
        mattkrause 6y ago
        Ha! You’re clearly taking the mickey but... Some things only/obviously make sense over pairs of items. For those, go with a literal 2 for x,y in itertools.combinations(obj, 2): if is_overlapping(x,y): raise OverlapError(x,y) However, sometimes the subset’s size just happens to be two (but you might change it), maybe a constant would be good. Ditto if you’re doing some math where there are “real” 2s that are part of a formula and incidental ones that are due to the subset size. For example: subset_size = 2 for subset in itertools.combinations(X, subset_size): mse.append(sum((subset - target)**2)/subset_size) is a bit more flexible and more clear than using all 2s, IMO, and at very little cost (YAGNI blah blah, but I think that’s an argument for not making a whole configuration system that lets you set the size at runtime).
- crazygringo 6y ago
  I'm not sure where the misunderstanding is coming from. Yes, of course if you have a formula that involves dividing by 2 or something you just include the number directly. But anything that's a magic number, even if only used once, is better to have a named constant defined. The name of the constant becomes the documentation itself. The other reason is that if someone needs to come in and maintain the code and change a magic number for whatever reason... if it's a defined constant, they know it should only need to be changed in that one place, assuming all magic numbers are defined in a sufficiently wide scope. If it's just a number, they have to search the whole codebose for all instances of that number, and investigate each and every one to see if it needs to be replaced as well or not.
  - benibela 6y ago
    >Yes, of course if you have a formula that involves dividing by 2 or something you just include the number directly. Or replace it with >> 1
    - deleted 6y ago
      [deleted]
pmiller2 6y ago
The spirit of the advice is mostly correct. However, there are reasons to use numeric literals in code. One is the PRNG example given downthread. Another is one I recently wrote. I have some code where I needed to calculate what the 1st percentile of a list of numbers was. Since it doesn't make a lot of statistical sense to calculate a 1st percentile of fewer than 100 numbers, I inserted a condition like if len(numbers) < 100: # skip the calculation and included a comment stating that 100 is explicitly not a magic number here. By that, I meant that you'll never want to change this 100, so, why bother obfuscating it behind a name? It's perfectly readable, if you understand why the calculation is skipped in the first place. If you don't understand why the calculation is skipped, giving it a name like TOO_FEW_NUMBERS_TO_CALCULATE_1ST_PERCENTILE isn't really going to give you much insight into why the calculation is skipped, anyway.
- necovek 6y ago
  You'd name it MINIMUM_NUMBERS_FOR_1ST_PERCENTILE (your name was for the entire if). To me "if len(numbers) < MINIMUM_NUMBERS_FOR_1ST_PERCENTILE" still reads better and requires me not to need to refer to comments, which I fall back on when I do not immediately get the code. So maybe the risk is not lowered, but readability is still improved. (Oh, and I wonder what do you return for a list of all equal numbers when there are more than a 100? If it's that number, why would suddenly going from 100 to 99 change that? ;)

by ztjio 6y ago

klyrs 6y ago

> I'm going to assume you're not deliberately trying to misunderstand him for the sake of a hot take, because, that would be pretty lame. I'll admit, my example was overwrought with comedic intent, but I stand firm on this issue. Use a magic number twice? Yeah, go ahead and put that numeric literal somewhere convenient (but you still have a numeric literal) to the consumers of it and easy to find by your readers (not a top-level magic_numbers.h) I'm a mathematician. The fear of numbers in code is an affront to the domain that I work in. YMMV. I put a lot of work into documenting my code, but for the love of pete, 2 is not a magic number: my example was no more overwrought than OP's rule.

mattkrause 6y ago
It depends on why you're using the 2. In some cases, I'd say that 1 can be a magic number. For example, it would better to write fprintf(STDOUT, output_string) than fprintf(1, output_string) However, 0 and ±1 have a special role in picking out the first/next/previous elements of a sequence (and related things), which is so common that it'd be silly to insist on defining POSSIBLE_NEXT_INDEX and POSSIBLE_PREDECESSOR. That said, people do seem to love Python's itertools, so....maybe that's where we're headed. I had somebody aggressively complain about the readability of the all-pairs for-loop, which I thought was basically standard. for(auto i=0; i<N-1; i++) for(auto j=i+1; j<N; j++) process(X[i], X[j]) The flip side is that I don't think you always need to do this. For example, this is silly: double triangle::area() const { const double NORMALIZATION = 1/2.0; return this->base * this->height * NORMALIZATION; } The point is just to make it clear why that particular number is being used and where it came from.
- rmtech 6y ago
  The all pairs loop is unnecessarily low level, and it would be hard to spot a mistake there. It would be great if there was a way to say for i < j <= N as this is super clear.
  - mattkrause 6y ago
    I guess so. I don't think it's atrocious: it looks like iterating over the upper/lower triangular part of a matrix to me, which is fairly common (in some domains, I guess). Plus, I like that the double for-loop indicates slowness. However, Python has itertools.combinations(X,2) and Julia has IterTools.subsets(X,2) if that's what you want.
    - klyrs 6y ago
      I think you mean # see __magic_numbers.py itertools.combinations(X, number_of_things_in_a_pair)
      - deleted 6y ago
        [deleted]
      - mattkrause 6y ago
        Ha! You’re clearly taking the mickey but... Some things only/obviously make sense over pairs of items. For those, go with a literal 2 for x,y in itertools.combinations(obj, 2): if is_overlapping(x,y): raise OverlapError(x,y) However, sometimes the subset’s size just happens to be two (but you might change it), maybe a constant would be good. Ditto if you’re doing some math where there are “real” 2s that are part of a formula and incidental ones that are due to the subset size. For example: subset_size = 2 for subset in itertools.combinations(X, subset_size): mse.append(sum((subset - target)**2)/subset_size) is a bit more flexible and more clear than using all 2s, IMO, and at very little cost (YAGNI blah blah, but I think that’s an argument for not making a whole configuration system that lets you set the size at runtime).
crazygringo 6y ago
I'm not sure where the misunderstanding is coming from. Yes, of course if you have a formula that involves dividing by 2 or something you just include the number directly. But anything that's a magic number, even if only used once, is better to have a named constant defined. The name of the constant becomes the documentation itself. The other reason is that if someone needs to come in and maintain the code and change a magic number for whatever reason... if it's a defined constant, they know it should only need to be changed in that one place, assuming all magic numbers are defined in a sufficiently wide scope. If it's just a number, they have to search the whole codebose for all instances of that number, and investigate each and every one to see if it needs to be replaced as well or not.
- benibela 6y ago
  >Yes, of course if you have a formula that involves dividing by 2 or something you just include the number directly. Or replace it with >> 1
  - deleted 6y ago
    [deleted]

pmiller2 6y ago

The spirit of the advice is mostly correct. However, there are reasons to use numeric literals in code. One is the PRNG example given downthread. Another is one I recently wrote. I have some code where I needed to calculate what the 1st percentile of a list of numbers was. Since it doesn't make a lot of statistical sense to calculate a 1st percentile of fewer than 100 numbers, I inserted a condition like if len(numbers) < 100: # skip the calculation and included a comment stating that 100 is explicitly not a magic number here. By that, I meant that you'll never want to change this 100, so, why bother obfuscating it behind a name? It's perfectly readable, if you understand why the calculation is skipped in the first place. If you don't understand why the calculation is skipped, giving it a name like TOO_FEW_NUMBERS_TO_CALCULATE_1ST_PERCENTILE isn't really going to give you much insight into why the calculation is skipped, anyway.

necovek 6y ago
You'd name it MINIMUM_NUMBERS_FOR_1ST_PERCENTILE (your name was for the entire if). To me "if len(numbers) < MINIMUM_NUMBERS_FOR_1ST_PERCENTILE" still reads better and requires me not to need to refer to comments, which I fall back on when I do not immediately get the code. So maybe the risk is not lowered, but readability is still improved. (Oh, and I wonder what do you return for a list of all equal numbers when there are more than a 100? If it's that number, why would suddenly going from 100 to 99 change that? ;)