Why is initializing a struct field with a default value considered a 'magic-number' by clang-tidy? [closed]

Linter warnings don't exist in isolation; they are here to help you make meaningful improvements. In this case, important information is missing from the code, and the linter warning is there to give you chance to remedy the situation.

In the case of port 1993 you need to ask where the 1993 comes from, or why 1993 and not some other number. The defaultTcpPort name addition by itself is not helpful or much of an improvement. Is this the arbitrary port you use internally for your service by default? Indicate that as part of the name. Is it the common default port for a certain application, like SNMP or NetStat? Then name the application as part of the variable declaration. In this way, you now include important information as part of the code, which can assist with later maintenance.

You also asked about IP addresses. This doesn't trigger because the IP address is declared as a string, and holding magic values is often exactly what string variables are there to do... raising linter warnings for strings could create too much noise to be helpful. But that doesn't mean you're wrong to ask the question and be aware of what this variable is doing.

In fact, 127.0.0.1 is a special IP address. Think of if like the 1 identity value for numbers... it's not exactly arbitrary. Having a special varialbe declared here may well be a good idea. But if you were indeed to make a constexpr variable ("constant variable" always makes me chuckle 😉) for the value, you would NOT just name it defaultIpAddress. You would name it something like localhostIPv4 or IPv4HomeAddress... use a name that has meaning. Then you can assign that constant as the default, and it becomes clearer in code what is happening: you are assigning the home/loopback IP4 address as the default value.

But what if the default changes later on? Then you can create and assign a different constant to the DefaultSettings::tcpPort or DefaultSettings::tcpIpAddress variable.

Should we not have an exception for initializing default values in a struct?

NO.

At least, not generally. But imagine we emphasize the "initializing default values" as part of the definition for the struct. This seems to be what you're doing here, where the entire thing is meant to be constant, or at least immutable, and used only for reference elsewhere. That is, holding magic values (and consolidating/isolating many of them to this one place for easy reference/adjustment) is the primary purpose of this struct. In that situation you might have a case.

The problem is the linter does not (and can not) know that.

Perhaps as LLM/AI tools advance we'll eventually create linters capable of inferring this type of meaning. That is not yet the current state of the art. Today, things are more based on grammar syntax than semantic meanings. It only sees "struct" and can't infer other meaning from names or how it's used.

With that in mind, there are a number of patterns here you could choose to help with this, though often it's also a case of architecture overload. One possibility is loading the values from a config file, environment variables, etc at startup, possibly via a dependency injection mechanism to improve testing.

While not being an expert with the clang compiler and checks, I will try to answer with my own experience.

Short answer: No. Your second example (see below) is clearly more robust even if more verbose.

struct DefaultSettings { static constexpr std::string_view defaultIpAddress = "127.0.0.1"; std::string tcpIpAddress = defaultIpAddress; static constexpr uint16_t defaultTcpPort = 1993; uint16_t tcpPort = defaultTcpPort; } 

Long answer: Your question actually refers to the issue involved with "magic numbers". Let's consider the following:

void connectToServer(const DefaultSettings &serverSettings) { if (serverSettings.tcpPort == 1993) { /// treat the default port, might be an invalid port with a straight return } else { /// actually connect to the server } } void checkServerConnection(const DefaultSettings &serverSettings) { if (serverSettings.tcpPort == 1993) { /// treat the default port, might be an invalid port with a straight return } else { /// actually connect to the server } } 

It is clear that, if your default port configuration is changed for any reason, you need to manually change every line involving the default port. Even for small projects with +10k lines of code it is intractable (at least from my point of view). This "coding style" is highly subject to create future bugs. Therefore, one prefers the following where only one line must be changed if needed:

void connectToServer(const DefaultSettings &serverSettings) { if (serverSettings.tcpPort == DefaultSettings::defaultTcpPort) { /// treat the default port, might be an invalid port with a straight return } else { /// actually connect to the server } } /// and so on... 

As a rule of thumb, ALWAYS define hard coded values, e.g char, int, float, and by extension strings into constant declarations. I've already seen people writing my_array.size() == ZERO_SIZE_ARRAY instead of my_array.size() == 0 (in other languages though). This is a trade-off between readability and robustness.

Remark on your code:

I would suggest to not encode the default value directly into the struct/class but rather do:

struct Settings { std::string tcpIpAddress; uint16_t tcpPort; } constexpr const char* DEFAULT_TCP_IP_ADDRESS = "127.0.0.1"; constexpr uint16_t DEFAULT_TCP_PORT = 1993; constexpr Settings DEFAULT_SETTINGS{ DEFAULT_TCP_IP_ADDRESS, DEFAULT_TCP_PORT }; 

Or something along the line. A struct/class skeleton has the purpose to declare things and not to define them: in other words, a struct/class carries values but does not set them (if we do not consider the methods attached to them, I am a C speaker). This way of coding could also allow you to load dynamically, from a file for example, the default values more easily (but remove the constexpr).

Note:

The reason why clang (and probably other compilers) does not apply an equivalent checks for strings (if this does not exist already), is probably because std::string_view is a class and the compiler does not make the difference between a default constructor std::string tcpIpAddress; and std::string tcpIpAddress = "my specific address" , taking into account the heap allocation. (I might be wrong, others are welcome to correct me).