Hacker News Reader

Comment by bryanlarsen | Hacker News Reader

I'd go even further and disagree with this statement too:

"By hard coding, I don’t mean you should spread magic numbers and strings across your project’s source code."

The first time you use a magic number or string, put it in directly. It's much easier to find and change in its home environment, and it keeps your code simpler and easier to understand.

Of course, once you use that magic number somewhere else, then you should introduce a constant -- you don't want to get in a situation where somebody changes it in one place but not the other. But until then, don't complicate things.

If you feel that I'm a heretic, then please define the constant where you use it (when it's only used in one place), rather than putting it in a header.

View on HN

I think there's a difference between:

embedded in the code, and

   const int maxRetries = 5;

at the top of a file. Both are still hardcoded, but there's at least context for the number.

I'd far rather have the 5 embedded in the code. It should be obvious from the code that the 5 is the number of retries, and it's often important to know at least the magnitude of the number when reading the code. Don't make me hunt for the number unless you have a good reason for doing so.

If the name is required to understand the code, then putting your "const int maxRetries = 5;" in the code is better than a comment that says the same thing. Just don't put it at the top of the file. Put it on the line before you use it.

Factoring out constants has a nice effect of turning your header into a sort of declarative program, a quick summary of the interesting behavior choices the program makes, a guide to understanding the program at a glance.

With numRetries=5, you can see he program makes retries. Withjust 5, that fact is hidden away in a method parameter somewhere.

And unless your IDE is ed+cat, there is no hunting needed to jump to a definition of a method. Having layers of generality makes code easier to read.

ed + cat seems like an odd combination: why not use just ed (it has a print command, p)?

> And unless your IDE is ed+cat, there is no hunting needed to jump to a definition of a method. Having layers of generality makes code easier to read.

I completely disagree; adding extra indirection/generality always makes your code harder to understand. Adding meaningful/intuitive abstractions makes it significantly easier though.

My pet peeve is Java interfaces that only exist because you need to be able to mock the object in testing, but otherwise only have one real implementation. Especially when the name gives no indication that it's an interface, so you only find out after you pressed F3.

If you put the const declaration just before you use it and then you or someone else uses it somewhere else, you are going to have an even harder time finding it. If it needs a const, declare it at the top so I know where to look.

You say it should be obvious what a value embedded in code is. OK, but for one reason or another it often is not. So why not have your coding standard be name constants well at the top of the file or function. I think that is more consistently readable and reusable.

This is silly. If you don't have a single constant, you could have the situation where one piece of code had a maxRetry of 5, and another with a maxRetry of 2. Good luck trying to figure out why things are failing when they should retrying 5 times.

The problem is you might (sometimes should) have different retry counts depending on what you're retrying. So MAX_RETRY (or whatever) isn't actually that meaningful when you run into the constant. It might actually be misleading.

This is solveable with extra verbosity.

  # no:
  MAX_FAILS = 3
  MAX_RETRIES = 5

  # yes:
  MAX_API_CONNECTION_RETRIES = 3
  MAX_COMPLEX_GRAPH_SOLVING_RETRIES = 5

Apologies for terrible names, but the point is that you should name the constant different things, so that it's clear what __kinds__ of retries you are capping.

I think if you can't reliably find all occurrences of a value then it shouldn't be hardcoded - 6.022e23 is easier to find in a large source file than 5. If you work in a statically typed language with a decent IDE you can usually see the constant value just by hovering over the identifier.

I'm guessing there are different tradeoffs in dynamically typed languages since the hardcoded number will also identify the type.

I'm all for extracting constants, but not (always) to the top of the file: why not put them as close as possible to where they are used - especially for one time uses?

Naming and being at the top of the file are separate things.

If it needs a name, name it where it's used!

I think of the "no magic constants" as a primitive precursor to the DRY principle. It was a good approximation for its time, but now we know better.

I think defining the constant the first time you use it (if you're assuming you might only use it once) may be a reasonable solution to this, but if you plan on anyone else to ever be looking at your code I feel like magic numbers should absolutely be attached to some sort of name.

If someone were working on a different section of code and came upon a place where they needed maxRetries, but there was only a 5 defined somewhere, how would they know that it was already used once? Defining constants for anything that may be used multiple times saves the time of knowing/combing through every other section of code that could have already defined it.

I agree with this too. Pulling constants out to the top a source file is often a mistake. It's useful if someone is looking at the source file to find the constant, but usually it's more likely that they're reading the code to understand it, and then need to know the value of the constant. In these cases it can be better to have a constant but define it close to where it is used.

Personally, if I'm reading the code, seeing `maxRetries` is more meaningful than 5. `maxRetries` both abstracts away the 5 (which could change at whim), and makes it so I don't have to parse the surrounding code to identify exactly what that loop is doing (unless it's not working, in which case I have to parse that loop anyways).

It also encourages you (for better or worse) to use `maxRetries` in multiple places.

The value of 5 is, in the long run, meaningless. It could have easily been 3 or 10.

But what if maxRetries is 0 or 1000? (I've seen both). That really changes the meaning of the code you're reading.

1000 doesn't change the meaning at all - it's still retrying a block of code some finite number of times. 0 does change it a bit, but only in that the code is not being executed, not that there is suddenly no retry logic which exists in the code.

IDEs make it trivial to find the value of a constant. IDEs like IntelliJ and Chrome developer tools will even show you them inline while debugging.

Reply on news.ycombinator.com