cppbestpractices icon indicating copy to clipboard operation
cppbestpractices copied to clipboard

A case against UPPERCASE constants

Open sasq64 opened this issue 5 years ago • 3 comments

My argument is that avoiding all-uppercase for all non-macros avoids those hard to understand errors you get when you use a constant or enum value that was already defined in some external header.

Ever had to do #undef ERROR after including Windows.h ?

sasq64 avatar Nov 23 '18 12:11 sasq64

I confirm the ERROR issue was annoying enough recommend no lower-case only names:

https://github.com/valhalla/valhalla/blob/4fb5c6e58b2cf7628f3c7311cf2fa13bc0155023/valhalla/midgard/logging.h#L4-L6

https://github.com/valhalla/valhalla/blob/4fb5c6e58b2cf7628f3c7311cf2fa13bc0155023/valhalla/midgard/logging.h#L31-L33

mloskot avatar Nov 23 '18 13:11 mloskot

Sure, there's a case against upper case constants, but how else are you going to specify global scope immutables immediately? You need to come up with a solution, because it is very annoying trying to figure out where a variable I'm looking at comes from only to find out it was defined in a header file I'm including, and isn't a part of the class I'm looking at.

Enums can be upper camel case because you think of the values almost as types in their own right, but using enum classes disambiguates where they come from anyway, so signifying with caps isn't needed. Global constants, on the other hand, need to be something that's signified exists outside of the current scope and isn't going to be modified underneath you. Capitalization was the easiest way to signify this, but I agree, this does conflict with macros. There needs to be something to replace it if you are going to make that argument.

Cazadorro avatar Nov 27 '18 19:11 Cazadorro

Actually I have an idea for a replacement. Given the Always Use Namespaces rule, how about we change the rule for global constants to always scope with at least first namespace? ie:

namespace xyz{
    const int const_example = 3;
    ...
    class Abc{
    private:
        int m_baz;
    public:
        int foo(){
            int bar = m_baz * xyz::const_example;
            return bar;
        }
    }
}

This wouldn't apply to local constants, as you know exactly where they are defined. This could apply to class static constants used internally in a class, but regardless they are already forced to use the class namespace when used outside of a class. Non static class constants are essentially local constants, so again they wouldn't need to be scoped.

Now you know the global constant comes from a different scope, and you can understand that it is a constant by usage, rather than second guessing where it came from. Outside the namespace you already have to scope, so it doesn't create more code for the user of your API, just internally.

Cazadorro avatar Nov 27 '18 21:11 Cazadorro