34

I'm fairly new to software engineering, and so as a learning exercise I wrote a chess game. My friend had a look at it and pointed out that my code looked like

for (int i = 0; i < 8; i++){
    for (int j = 0; j < 8; j++){

while he insisted that it should instead be

for (int i = 0; i < CHESS_CONST; i++){
    for (int j = 0; j < CHESS_CONST; j++){

with some better symbol name that I can't be bothered to think of right now.

Now of course I do know to generally avoid using magic numbers, but I feel like since

  1. this number will never change;
  2. the name couldn't be that descriptive anyway since the number is used in so many places throughout the code; and
  3. anyone going through source code for a chess program should know enough about chess as to know what the 8 is for,

there really is no need for a symbolic constant.

So what do you guys think? Is this overkill, or should I just go with convention and use a symbol?

  • 46
    CHESS_CONST is way worse than just using the number 8, but a constant with a descriptive name would be an improvement. You say anyone should know what 8 stands for in the code, but this is not true. An integer literal without context could mean any number of things, like a number of moves, number of pieces on the board and so on. A descriptive name for the constant would make the intention clear and hence the code easier to understand. – JacquesB Apr 8 '17 at 8:20
  • 43
    Personally, what I find much more jarring than the magic number is the names i and j for the loop variables. I cannot for the life of me figure out which one is supposed to represent rank and which one is supposed to represent file. Ranks range from 1..8 and files range from a..h, but in your case, both i and j range from 0..7, so that doesn't help me see which is which. Is there some international letter shortage crisis I don't know about, or what is wrong with renaming them to rank and file? – Jörg W Mittag Apr 8 '17 at 8:21
  • 4
    Play devil's advocate for a second; Imagine reading somebody's chess code where they've used a magic number 8. Can you assume you know what it's used for? How can you be 100% sure? Is there any possibility it could mean something else? Wouldn't it have just been a bit nicer if you didn't even have to make an assumption? How much time might you spend tracing through the code to figure out whether your assumption is right? Would you spend less time if the code had been more self-documenting using a meaningful, insightful name instead? – Ben Cottrell Apr 8 '17 at 8:22
  • 16
    @JacquesB: Indeed. There's 8 ranks, 8 files, 8 pawns. Some chess engines use a points system where certain pieces, certain fields, and certain moves have points attached to them and the engine chooses the move with the highest score. 8 might be such a score. 8 might be a default search depth. It might be pretty much anything. – Jörg W Mittag Apr 8 '17 at 8:23
  • 9
    I'd consider using a foreach loop, e.g. foreach(var rank in Ranks). I'd also consider merging both loops into one where each element is a (rank, file) tuple. – CodesInChaos Apr 8 '17 at 11:52
102

IMHO your friend is right in using a symbolic name, though I think the name should definitely be more descriptive (like BOARD_WIDTH instead of CHESS_CONST).

Even when the number will never change through the lifetime of the program, there may be other places in your program where the number 8 will occur with a different meaning. Replacing "8" by BOARD_WIDTH wherever the board width is meant, and using another symbolic name when a different thing is meant makes these different meanings explicit, obvious and your overall program more readable and maintainable. It enables you also to do a global search over your program (or a reverse symbol search, if your environment provides it) in case you need quickly to identify all places in the code which are dependent on the board width.

See also this former SE.SE post for a discussion how to (or how not to) pick names for numbers.

As a side note, since it was discussed here in the comments: if, in your real program's code, it matters if the variable i refers to rows and j to columns of the board, or vice versa, then it is recommendable to pick variable names which make the distinction clear, like row and col. The benefit of such names is, they make wrong code look wrong.

  • 22
    I would like to add, if the OP had written a really great chess engine then wished to expand it to include 3D chess, or other variants, then which 8s need to change is going to be a challenge. – Steve Barnes Apr 8 '17 at 8:20
  • 32
    @SteveBarnes: I try to avoid such arguments, since it leads too easily to an reverted argument like "I think it is extremely unlike ever to make this program a different kind of game, so I can leave the magic number 8 where it is." – Doc Brown Apr 8 '17 at 9:47
  • 4
    @IllusiveBrian That's a classic "straw man" argument for two reasons: (1) just defining a constant doesn't mean the programmer still can't type "8" (whether by accident, design, or malice!) and (2) just because there is a constant BOARD_WIDTH = 8 doesn't make BOARD_WIDTH the correct constant to use at a particular place in the program. – alephzero Apr 8 '17 at 13:26
  • 3
    @Blrfl ...or will lead to overengineering the code. It's developer's responsibility to realize what can change in the future and code accordingly. Coding like requirements won't change causes problems, but the other extreme is no better. – Malcolm Apr 8 '17 at 14:01
  • 4
    @corsiKa When the loop variables correspond to physical axes, they should be named x,y or row,col or, for chess, file,rank. – user949300 Apr 9 '17 at 9:07
10

Ok, here are a few comments I have:

Getting rid of magic numbers is a great idea. There is a concept known as DRY, which is often misrepresented, but the idea is that you don't duplicate the knowledge of the concepts in your project. So if you have a class called ChessBoard, you could keep a constant called BOARD_SIZE or ChessBoard.SIZE attached to it. This way there is one sole source for this information. Also, this helps readability later:

for (int i = 0; i < ChessBoard.SIZE; i++){
  for (int j = 0; j < ChessBoard.SIZE; j++){

Even if the number never changes, your program is arguably better. Any person reading it knows more information about what the code is doing.

A bad name is worse than no name, but that doesn't mean that something shouldn't be named. Just change the name. Don't throw out the baby with the bath water. :p The name can be descriptive as long as you understand well what it is describing. Then, that concept can be used for multiple different things.

  • 8
    this seems to merely repeat points already made and explained in top answer – gnat Apr 8 '17 at 15:15
-7

What you really want is to eliminate ad nauseum references to constants, whether they be named or bare:

for_each_chess_square (row, col) {
  /*...*/
}

If you're actually going to proliferate the constant by repeating such loops and whatnot, it's best to stick with 8.

8 is self-describing; it's not a macro that stands for something else.

You Ain't Never Gonna (TM) turn it into a 9x9 chess program and if you ever do, the proliferation of 8 will not be the major difficulty.

We can search a 150,000 line code base for the token 8, and classify which occurrences mean what in seconds.

Far more important is to modularize the code so that the chess knowledge is concentrated in as few places as possible. It's better to have one, two, maybe three chess-specific modules in which a literal 8 occurs, than thirty-seven modules laced with chess-specific responsibility, referring to 8 through a symbolic name.

When or if this 8 constant becomes a source of tension in your program, you can easily fix it at that time. Fix real problems that are happening now. If you don't feel that you're hampered by that particular 8, go with that instinct.

Suppose that in the future you want to support alternative board dimensions. In that case, those loops will have to change whether they use a named constant or 8, because the dimensions will be retrieved by some expression like board.width and board.height. If you have BOARD_SIZE instead of 8, these places will be easier to find. So that is less effort. However, you must not forget about the effort of replacing 8 with BOARD_SIZE in the first place. The overall effort is not lower. Making one pass over the code to change 8 to BOARD_SIZE, and then another to support alternative dimensions, is not cheaper than just going from 8 to alternative dimension support.

We can also look at this from a purely cold, objective risk/benefit analysis. The program has bare constants in it now. If these are replaced by a constant, there is no benefit; the program is identical. With any change, there is a nonzero risk. In this case, it is small. Still, no risk should be taken without a benefit. To "sell" the change in the face of this reasoning, we have to hypothesize a benefit: a future benefit which will help with a different program: a future version of the program that doesn't exist now. If such a program is being planned, this hypothesis and its associated reasoning are bona fide and should be taken seriously.

For instance, if you're days away from adding more code that will further proliferate these constants, you might want to do away with them. If those instances of the constants are approximately all the instances that will ever exist then why bother.

If you ever work on commercial software, ROI arguments will also apply. If a program isn't selling, and changing some hard-coded numbers to constants won't improve the sales, you will not be compensated for the effort. The change has zero return on the investment of time. ROI arguments generalize beyond money. You wrote a program, investing time and effort, and got something out of it: that's your return, your "R". If by making that change alone, you get more of that "R", whatever it is, then by all means. If you have some plan for further development, and that change improves your "R", ditto. If the change has no immediate or forseeable "R" for you, forget it.

  • 13
    8 is NOT that self-describing, it's a number that could mean anything. And maybe they do want to do a 9x9 chess game, why not? If you have 150,000 lines of code, there is no way you're going to be able to memorize what each 8 means in the code, and even if you could, why would you? That's just a lot of extra hassle. As far as modularization, replacing 8 with something like NUM_RANKS doesn't hurt modularity, in fact, it helps because it makes the code easier to tinker with. – Jerfov2 Apr 8 '17 at 20:58
  • 4
    @Kaz I do that too. And then I introduce significant weird bugs because a handful of usages weren't what I thought they were, because it's hard to pay that much attention to a large number of replacements and also be correct every time. For that reason I avoid getting into this scenario in the first place, so I can't support advice that condones getting into it. – doppelgreener Apr 9 '17 at 18:51
  • 1
    "However, you must not forget about the effort of replacing 8 with BOARD_SIZE in the first place" - The time you'd spent scrutinizing over your code trying to figure what each 8 does in your program months from now would most likely exceed the time spent replace 8 with a meaningful module-level constant. – Christian Dean Apr 9 '17 at 18:56
  • 1
    @doppelganger That scenario is already here. I'm not saying, take a chess program which uses constants, and replace them with 8. Nor am I saying, "plan not to use constants in a not-yet-written chess program" – Kaz Apr 9 '17 at 19:11
  • 1
    @Kaz Why would you want to make yourself have to read all the code around the 8, and maybe sometimes get the context wrong when you could just spend an extra 3 seconds giving in a meaningful name? As for knowing the actual value of the constant, it is much easier looking up the exact value for a variable than trying to backwards-engineer what some magic integer in the code means – Jerfov2 Apr 11 '17 at 0:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.