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?

Peter Mortensen
  • 1,050
  • 2
  • 12
  • 14
danielvolchek
  • 459
  • 4
  • 5
  • 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 08 '17 at 08: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 08 '17 at 08: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 08 '17 at 08: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 08 '17 at 08:23
  • 1
    Yes, everyone that plays chess knows that 8 means the number of non-pawn symbols each side has control of. Anyone who thinks that 8 had any other meanings in chess is an idiot who doesn't really know chess. – Lie Ryan Apr 08 '17 at 10:41
  • 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 08 '17 at 11:52
  • 2
    Possible duplicate of [Eliminating Magic Numbers: When is it time to say "No"?](http://softwareengineering.stackexchange.com/questions/56375/eliminating-magic-numbers-when-is-it-time-to-say-no) – gnat Apr 08 '17 at 12:57
  • @LieRyan Well, you must be one because 8 is also the number of *pawn* symbols, and the length and with of the board. – Kaz Apr 08 '17 at 14:27
  • @Kaz: Don't let anyone fool you, the 8 is the number of non-pawn pieces is The One and Only True Obvious Interpretation(tm) for the number 8 in chess. The others are fake or lying, or heresy. – Lie Ryan Apr 08 '17 at 14:37
  • I would even consider to go a step further, and let the user change this value in case he want to. It should not be too hard (assuming you are write software for a PC, server or similar and not on a embedded device where every byte count) to allow that and it has no bad effect for the user who only use the default one. But you never know when someone find a use i do not think of (i do not see reason to avoid possibilities you did not think of, make software as generic as possible as long it does not increase the complexity) – 12431234123412341234123 Apr 08 '17 at 14:54
  • 1
    @LieRyan: You can have more than eight non-pawn pieces. – gnasher729 Apr 08 '17 at 18:14
  • 1
    @Jörg W Mittag: It depends on the intended audience. You are apparently familiar with written descriptions of chess, where the terms 'rank' and 'file' are, I think, standard nomenclature. Non-chess aficionados wouldn't know this. OTOH, to someone more familiar with math and 2D arrays, 'i' and 'j' are self-explanatory, and to a non-chess playing programmer, calling an index variable 'file' would be a source of confusion :-) – jamesqf Apr 08 '17 at 20:00
  • I have a side query: Which of the C like languages are you using? – ctrl-alt-delor Apr 08 '17 at 21:44
  • @jamesqf: I find it highly unlikely that someone who doesn't know how chess works would be able to write a chess engine. Like I wrote in my comment: I challenge you to tell me which loop loops over the ranks and which loop loops over the files. If they were named `rank` and `file`, it would be immediately obvious. If they ranged from `1..8` and `a..h`, it would be immediately obvious. But, they are called `i` and `j` and both range from `0..7`, so I have no idea. And someday, some poor maintenance programmer also will have no idea. By the way: I had to look those terms up on Wikipedia since … – Jörg W Mittag Apr 08 '17 at 21:45
  • 1
    … I am actually not a domain expert on chess. I only knew that there *are* terms for those, but I actually thought they are called "row" and "column". However, someone editing that code *is* likely to be a domain expert on chess, and will be familiar with those terms … and if he isn't, he can easily look them up. – Jörg W Mittag Apr 08 '17 at 21:46
  • 1
    @JörgWMittag As we have not seen the rest of this code, whether `i` and `j` are better or worse than longer names is hard to say IMO. You may be interested in this question on math.SE: [Why do mathematicians use single-letter variables?](https://math.stackexchange.com/questions/24241/why-do-mathematicians-use-single-letter-variables) – if you read through all the answers, you'll find a few points that are relevant in certain programming contexts too. – ShreevatsaR Apr 08 '17 at 22:19
  • **Its generally a good idea in almost all programming languages to have fixed values as constants with descriptive names.** From CSS to TSQL.scripts. – Mark Rogers Apr 09 '17 at 15:32
  • @Jörg W Mittag: Why do you think that? IIRC, it's a common enough assignment in programming classes. A developer could be given the moves as part of a project specification, without ever having seen chess literature. I certainly have no effing clue which is rank or file, but (as a programmer working in science & engineering) know that i is almost certainly the horizontal dimension of the board, j is the vertical. Adding a translation layer to convert indexing from 1-8/a-h would make it even less clear. What makes a descriptive name depends on the audience. – jamesqf Apr 09 '17 at 18:20
  • Shouldn't _"symbolic constant"_ be simply _"constant"_? – Tulains Córdova Apr 10 '17 at 01:05
  • @MarkRogers -- Unless you're doing mathematical / scientific programming. When I'm reviewing code based on a paper, short terse variables names are preferable to big long ones. And `for (int ii = 0; ii < 3; ++ii)` is infinitely better than is `for (int ridiculously_long_name = 0; ridiculously_long_name < NUMBER_OF_SPATIAL_DIMENSIONS_IN_THE_UNIVERSE; ++ ridiculously_long_name)`. – David Hammen Apr 10 '17 at 05:04
  • 2
    @DavidHammen - I totally disagree that short terse names are better than big long ones. They both suck. "Descriptive to the degree necessary" names is the goal. Unless you are working with i,j,k in 3D vector math, i, j or k are absolutely dreadful names. – Dunk Apr 11 '17 at 18:05
  • @Dunk matrix math is another instance where such variable names are convention (as they are the typical variable names for indices of a matrix). – JAB Apr 12 '17 at 17:56
  • @JAB - They are the convention when working with vectors. The i, j, k actually have a specific meaning and aren't just random letters used for variable names. – Dunk Apr 12 '17 at 19:50

3 Answers3

101

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.

Community
  • 1
Doc Brown
  • 199,015
  • 33
  • 367
  • 565
  • 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 08 '17 at 08: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 08 '17 at 09:47
  • The thing about the "it is extremely unlikely" argument is that while it may hold for specific cases like chess, it doesn't hold up generally. Requirements do change, often after development is done. Encouraging newcomers to build for the general, it-will-change case discourages superficial thinking about the problem and how to solve it and fosters good development habits, both traits I like to see in software engineers. – Blrfl Apr 08 '17 at 11:55
  • Another advantage is that it prevents typos - you only have to get the 8 right once (and it's very conspicuous), and then every time you type the constant the compiler can tell you if you mistyped it. – IllusiveBrian Apr 08 '17 at 12:58
  • 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 08 '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 08 '17 at 14:01
  • 1
    @Malcolm: Agreed. Superficial thinking is what you get from slavish adherence to one of the extremes. – Blrfl Apr 08 '17 at 14:11
  • 1
    O.K. answer, but does not address Jorg's excellent point: using `i` and `j` is **far** worse than using 8. – user949300 Apr 08 '17 at 16:40
  • 2
    @user949300: the OP asked for using a symbolic instead of "8", not for "i" and "j", so I took his example as some kind of pseudo code to illustrate his point, not as a code snippet he wants to be reviewed in full. For the latter, he should post on codereview.SE – Doc Brown Apr 08 '17 at 16:45
  • Doc - maybe this is more of a SO "meta" discussion, but if an answer is to be worth 28 points (as your's is at this moment) it should do more than just say "yes, use a constant" while ignoring other more important issues. YMMV. – user949300 Apr 08 '17 at 17:43
  • 3
    @user949300: I agree, my mileage varies. I don't think the answer will become better when I make it a long-winded analysis what else could be improved. Brevity is a virtue. – Doc Brown Apr 08 '17 at 22:37
  • And make sure to have one of them be WIDTH and the other HEIGHT - useful when you start making [different types of boards](https://en.wikipedia.org/wiki/List_of_chess_variants#Orthodox_rules_on_an_unorthodox_board). – corsiKa Apr 09 '17 at 02:28
  • 1
    @user949300 `i` and `j` are pretty much immediately recognizable as "the loop variables" in most c-style languages. Deviating from that norm is likely to cause more confusion than it's worth. Obviously this is something you should work out with a team on a team-by-team basis. – corsiKa Apr 09 '17 at 02:30
  • 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 09 '17 at 09:07
  • 1
    @user949300 I stand by my statement. `i` and `j` are obviously loop variables. Deviation from that may cause confusion. Work it out with your team. Only the Sith deal in absolutes. – corsiKa Apr 09 '17 at 16:02
  • 3
    @corsiKa: you are right and nevertheless missed the point. Inside the loop, can you tell which one of the two variables is for the columns and which for the rows by names i and j? This starts to matter as soon as the loop block contains more than one line where i and/or j are used. – Doc Brown Apr 09 '17 at 18:19
  • @alephzero how is IllusiveBrian's comment a straw man argument? I don't see it. Anyway it's totally valid to say that preferring constants in general helps to avoid typos. He never said it would avoid all kinds of mistakes. That is a straw man argument. – callum Apr 21 '17 at 08:36
11

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.

Kaz
  • 3,572
  • 1
  • 19
  • 30
unflores
  • 402
  • 2
  • 9
-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.

Kaz
  • 3,572
  • 1
  • 19
  • 30
  • 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 08 '17 at 20:58
  • Searching for token 8 could result in a large number of false positives. Did you have something other than straight text search (with option "Match whole word" or similar) in mind? – Peter Mortensen Apr 09 '17 at 08:56
  • @PeterMortensen Nope. Say we have 1000 hits for `8`. That's one out of every 150 lines in the code: a very unlikely "crazy eights" program.. I could whip through those 1000 matches in a couple of minutes in a Vim "quickfix" and eliminate most of the falses. Do that sort of thing all the time. – Kaz Apr 09 '17 at 15:14
  • 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 09 '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 09 '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 09 '17 at 19:11
  • @algerbrex Since the program already exists the risk of changing it already exists. You could change the wrong 8 with the new constant. Since the constant's value is 8, no bug will appear *now*. Later, someone will be fooled by the wrong constant. Still, other factors have to "conspire". Like you change this wrong constant to `board.width`, and there happens to be a `board` in the scope which can take a `.width`. – Kaz Apr 09 '17 at 19:11
  • @Jerfov2 *8 is NOT that self-describing, it's a number that could mean anything.* That is simply false. It is the whole *context* surrounding that number which carries the additional meaning, not the integer itself. For instance `listen(sock, 8)` means that `8` is the listening "backlog" of the socket. Likewise, `listen(sock, CHESS_BOARD_SIZE)` means that `CHESS_BOARD_SIZE`, whatever it is, is also the listening backlog of the socket. The former is more informative; we know the backlog's value. If you think you can maintain code just by constant names, and ignore context, you're wrong. – Kaz Apr 10 '17 at 19:04
  • 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 00:57
  • @Jerfov2 You can rarely avoid reading the code around anything you touch. You can't just blindly do a search and replace on `BOARD_WIDTH` or what have you. At the very least you have to look at what file and function you're in. Whether a "naked 8" is "bad" depends on many factors. If the surrounding code is very clear with good use of other good identifiers, that may well rescue it. OTOH that function with those loops that walk the chess board is called `quux8` and the chess board data structure is called `b`, then those factors will make it hard to understand what that `8` is for. – Kaz Apr 11 '17 at 02:03