2

For example, consider I have constants VOL_MIN and VOL_MAX, which is used inside 1 function only:

public void setVolume(int val){
    final int VOL_MIN = 1;
    final int VOL_MAX = 10;
    val=Math.max(VOL_MIN,Math.min(val,VOL_MAX));
    //some other code
}

Originally I think everything should be accessible only when I need it, and VOL_MIN and VOL_MAX are used by setVolume() only currently, so I put them in the function.

But after some days, when I start forgetting what I wrote, I found it is hard to maintain: I need to search "10" to find where the constants defined, which works as if magic numbers.

So my question is, should the scope of constants be narrow as possible? Or should I define them in a easy-to-find place such as class static constants or inside a individual AppConstant class even though I can access it when I don't need?

4

My preference falls in the middle: I define constants within the class/module that uses them, but in an obviously static or "initialization-time" place, not in an individual method.

Why I don't define constants within methods

I work primarily in JavaScript where, in theory, if you define the constant in a method that declaration is run every time the method is called.

// Created when the module is loaded
const VOL_MIN = 1;

function setVolume(val) {
  // Created every time setVolume is called
  const VOL_MAX = 10;
  // ...
}

In practice modern interpreters probably try to help with this, but it's nice to be sure. In Java the situation is probably similar, but according to this answer static final variables will definitely be inlined by the bytecode compiler (not the JIT compiler) which might be a more compelling reason to define your constants on the class, rather than inside a method. In a compiled language it's up to the compiler, but they're probably optimized out wherever you put them.

From a style and change management perspective, I find constants tend to be tied to business rules or configuration, and thus in a growing project they're rarely confined to a single method for long. Suppose your setVolume method is on a Radio class. Conceptually, the minimum and maximum volume is a Radio-layer concern, and might be needed later for rendering UI, creating some sort of linear fade, etc.

Why I don't define constants in a Constants file

Besides not being a great organization strategy, dumping constants into a Constants file violates the principle that code that changes together should live together. Think of it in terms of compilation dependencies - if you have a Constants file and you change the value of one constant, every class in your code that uses a constant needs to recompile. If your constants live in the modules that use them, only the affected module (and its downstream dependencies) recompiles. On large projects, decisions like this start to have a big impact on development time.

| improve this answer | |
  • Surprisingly detailed. This is a very good answer, looking at the broader picture, evaluating the issue from several different points of view (compiler optimisations, business rules changes). Kudos! – Mael Mar 9 '18 at 8:02
  • Tempted to down-vote this, but will just comment instead: your reasons for "Why I don't define constants within methods" is a classic case of premature optimisation. This is not a good reason for avoiding constants in functions. – David Arno Mar 9 '18 at 9:32
  • That's a fair criticism - but it's an extremely low-cost thing to do as premature optimizations go. It also doesn't change my style preference - if the constant is conceptually tied to the module rather than the function I'd rather see it live at the module level. – Brad Buchanan Mar 9 '18 at 18:00
2

The answer to such a question is, as ever, "it depends".

If those constants are only used by setVolume, then by putting them in that function, you keep the scoping as narrow as possible and minimise any coupling. Their purpose is apparent, despite VOL_MIN containing almost no information on what it does. I can guess that refers to some minimum volume that the function will accept.

If the constants are out in a global constants file, then they are easier to find. But I then have no idea what they are for, or what might use them (though IDE's tend to help with the latter). VOL_MIN is now meaningless. So if you do expand the scope, the names need to improve hugely, to contain as much of: what sort of volume (eg litres or loudness); the full word "minimum", not an abbreviation; and, if possible, why that limitation exists.

I disagree that these are magic numbers in all but name. The key is they have names, thus they have context. 10 is just a number. MaximumSpeakerVolume contains information about that number.

I would advise keeping the scope of everything as narrow as possible, always. But if keeping constants really does cause you problems, then expand the scope. Don't make life hard for yourself just because you feel a sense of duty to follow guidelines. Be pragmatic and be efficient, as well as being "good practice" disciplined.

| improve this answer | |
1

Your question has multiple aspects. (BTW: I'm assuming your code to be Java, so my answer covers some technical aspects specific to that language)

Who needs these values?

You need them inside the setVolume() method. But you'd better communicate the limits of val to a potential user, so he isn't surprised that setVolume(100) has the same effect as setVolume(10). You should do that in a Javadoc comment, and maybe by making the limit values accessible. So it might make sense to make them public static final fields of your class.

Where can you declare them to really become constants?

In Java, declaring a variable final just means that, after initialization, you promise not to change its value later. It still is a variable to be allocated on the stack, visible in the debugger and so on (of course, compilers and the HotSpot engine are quite clever, so maybe they'll be able to detect that VOL_MIN is just meant to be another name for a literal 1).

Without relying on compiler optimizations, placing such a "constant" (a final variable) into a method means that every time you enter the method, a word on the stack is allocated, the initial value is computed (not in your case, as it's a literal integer) and copied into the stack word. So, using such a "constant" comes with a (tiny, depending on the level of optimization) performance penalty.

In contrast, a static final field (declared outside any method) is initialized only once when loading your class, and the language specification allows the compiler to replace usages of the field with just the literal value. And from personal experience I know that that happens. So static final fields don't have a performance penalty over plain literals.

Scope

So, where's the place to declare your constants?

  • Inside the method where they're used, they don't really become constants, but you probably won't notice the difference. Use that if you're concerned about tightest scope possible. But please make sure that they're really not useful for the outside world.
  • As private static final fields of your class, they're visible to all methods inside your class, so the scope is slightly larger. And they become real constants without a performance penalty over using the values as literals.
  • As public static final fields of your class, they become visible to all users of your class, so the scope is quite open. They become real constants without a performance penalty over using the values as literals. Being part of your class shows that these constants are related to properties of your class.
  • As public static final fields of a "constants" class, they're visible to all users, so the scope is quite open. They become real constants without a performance penalty over using the values as literals. Being in a class unrelated to your setVolume() method makes it hard to see the relationship with this method.

Summary

If you decide that the values aren't needed outside the setVolume() method, I'd go for private static final fields or (personal opinion) just use the plain literal values (in such a short, simple method, I don't see a readability advantage in introducing the names).

If you decide that the values are useful also for users of the setVolume() method, I'd use public static final fields in the same class.

If the values affect multiple methods in multiple classes, place them in a "constants" class.

Whatever you do, introducing a name for a literal value shouldn't take away information, so check the various possibilities with your favorite IDE to see if it shows the value when hovering over references of the constant.

| improve this answer | |

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.