2

For example, does it make sense to refactor the following code:

a = a * 2;

as:

const int INT_TWO = 2;

// ...

a = a * INT_TWO;

My question hinges on the fact that the new constant conveys no extra meaning (as opposed to calling it, say, FOOBAR_FACTOR).

Fa773N M0nK
  • 151
  • 7
  • One thing I can think of is that the code becomes a little more DRY due to this, because if the constant has to be changed, it can be done so from one place. In the former scenario, such changes may be quite difficult (requiring crafty find-and-replace). – Fa773N M0nK Apr 15 '20 at 15:58
  • 7
    IMO, `INT_TWO` makes no sense (and you have to pray that it doesn't get 'refactored' into `const int INT_TWO = 3;`). `FROBNICATION_FACTOR = 2` can make sense if that's what that 2 represents. – Mat Apr 15 '20 at 16:08
  • 6
    I can't see any legitimate reason why you should refactor expressions by introducing meaninglessly named constants. A constant must surely have meaning, therefore a name should be selected which expresses that meaning. – Steve Apr 15 '20 at 16:24
  • 3
    It doesn't make sense to use meaningless names for anything. `a` is just as meaningless a name as `INT_TWO` – mmathis Apr 15 '20 at 16:29
  • 2
    Related: [Are all magic numbers created the same?](https://softwareengineering.stackexchange.com/questions/266717/are-all-magic-numbers-created-the-same) – Doc Brown Apr 15 '20 at 17:10
  • 2
    I don't know why you would do this unless you're just spit balling stuff. You can start out with this while problem solving, but you should certainly refactor it to something meaningful. Meaningless names make future debugging extremely difficult. – WhyAyala Apr 15 '20 at 19:26
  • INT_TWO Needs to be hit with an RKKV. Nuke from orbit means you're way too close. – Loren Pechtel Apr 16 '20 at 02:44
  • I wanted to comment that I observed this in a legacy code I had to work on. I believe they are using (meaningless) named constants, when they should have been using Enums. The code base is C#. – Fa773N M0nK Apr 17 '20 at 05:29
  • I see no question in this ("does it makes sense to do something useless" is as pointless as the constant itself). There may be a question but this isn't it. Is it that you heard "using constants is good" and made up a silly example to challenge the statement? If so, a better question would be "How are constants helpful?" Anyway, I do not think there is much in it for anyone the way it is put now. – Martin Maat Apr 22 '20 at 18:27

2 Answers2

18

No this is utterly pointless. Don't just extract literals to named constants without good reasons.

But do consider ways to explain why the a value has to be doubled in that context. That could involve:

  • a function name that explains the purpose of doubling, using terms from the problem domain. For example:

    function exponential_backoff(task, delay):
  while task() is not success:
    sleep(delay)
    delay = delay * 2

  • a comment explaining the reason for doubling

    // The backoff factor should increase the delay between retries.
// A factor of at least two guarantees that the total load stays roughly constant
// when the number of clients launching failing requests increases at a constant rate.
delay = delay * 2

  • extracting the factor 2 to a constant or variable if it isn't obviously and necessarily always going to be 2.

    const BACKOFF_FACTOR = 2
delay = delay * BACKOFF_FACTOR

The risk with naming variables or constants after themselves is that they might be changed in the future. For example:

const POINT_EIGHTEEN = 0.20  // actually a VAT rate but can't refactor
const TWO = 3

This is especially risky for constants that are extracted incidentally, and possibly have different meanings in different parts of the code. On the other hand, some constants are truly fixed and are not going to change, e.g. PI, KIBIBYTE = 1024, or SPACE = ' '. Apply your common sense to figure out what makes sense to name, and what doesn't.

amon
  • 132,749
  • 27
  • 279
  • 375
  • What about [Zero](https://docs.microsoft.com/en-us/dotnet/api/system.decimal.zero) or [Zero](https://docs.microsoft.com/en-us/dotnet/api/system.timespan.zero) or [Zero](https://docs.microsoft.com/en-us/dotnet/api/system.numerics.biginteger.zero) or [One](https://docs.microsoft.com/en-us/dotnet/api/system.numerics.biginteger.one) or [MinusOne](https://docs.microsoft.com/en-us/dotnet/api/system.numerics.biginteger.minusone) – JimmyJames Apr 22 '20 at 17:44
  • @JimmyJames Many of those constants are superfluous, in my opinion. There's some justification for offering common values for objects like TimeSpan or BigInteger though. (a) The Zero TimeSpan is a good null object. (b) Constant pooling is good, but .NET can't fully do that when the classes have constructors. Ultra-common constants like zero or one are an adequate compromise. – amon Apr 22 '20 at 20:49
  • Right which I think kind of puts a small qualifier on 'utterly pointless'. I think it's a lot better to have code like `if value.equals(ZER0)` than `if value.equals(new BigInteger("0"))` or similar. If we agree on that, then specifying your code continually checks to see if a BigInteger value is equivalent to 2, it seems rational to create a named constant for that even if there's no better name for 2 in that algorithm. – JimmyJames Apr 23 '20 at 14:07
  • @JimmyJames In that example, if there is no meaningful name, I prefer `new BigInteger(0)` since it's clear immediately value is a BigInteger, not a byte or a float. – user949300 May 02 '20 at 00:28
  • @user949300 How would it be different for any named constant? Are you saying you would never use named constants for this reason? – JimmyJames May 04 '20 at 15:30
  • @JimmyJames I had interpreted your code snippet to be in a Unit Test, where I would generally not create a named constant. If that is "real" code, then I agree with you that the BigInteger(2) should have some meaning and be named. – user949300 May 04 '20 at 23:19
9

Depends

If 2 has a meaning, like "number of pints on a quart", then yes. e g.

pints = quarts * PINTS_PER_QUART

So, the real question is, why are you multiplying a * 2? If giving 2 a meaningful name helps understand the code, you should do so.

user949300
  • 8,679
  • 2
  • 26
  • 35
  • 3
    His question was whether it makes sense to use meaningless constants, not meaningful ones. – Steve Apr 15 '20 at 16:21