The constructor of my class initializes the first 3 fields in the initializer list, But for the salary field I'm required to use void calculateSalary() function. (professors demand) The code works perfectly fine, but I'm getting a warning: Clang-Tidy: Constructor does not initialize fields: salary I'm having difficulties understanding why I'm getting this warning since I can Initialize the salary in the body of the constructor with an r-value without getting an error e.g., salary = 5*year; and the function I'm calling does exactly that. I'm not sure if this is a redundant warning that I can ignore, or might this affect my code in some way?

class Employee
{
private:
    std::string name;
    std::string surname;
    int year;
    double salary;
public:
    Employee(int year, std::string name, std::string surname)
            : year(year), name(std::move(name)), surname(std::move(surname))
    {
        calculateSalary();
    }

    void calculateSalary()
    {
        salary = 2310   2310 * year * 12 / 100.0;
    }
};

CodePudding user response：

This seems to be a warning from clang-tidy going by the wording. As you can see here the full diagnostic is:

<source>:11:5: warning: constructor does not initialize these fields: salary [cppcoreguidelines-pro-type-member-init,hicpp-member-init]
    Employee(int year, std::string name, std::string surname)
    ^

If you move the salary = ... statement directly into the constructor instead of calling a function, you get a similar but different diagnostic:

<source>:14:9: warning: 'salary' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
        salary = 2310   2310 * year * 12 / 100.0;
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Both of these refer to the C core guidelines, a set of basic recommendations for writing C .

Here is clang's documentation for the first diagnostic and here for the second. Both have links to the C code guidelines.

In the first link it is documented that the first check gives false positives if assignment of a member happens through a function call instead of by direct initialization/assignment in the constructor.

As you can also see in the links, the recommendation of the guidelines which both diagnostics warn about if not followed, is to initialize all members in the constructor's member initializer list or since C 11, if it makes sense, in a default member initializer in the class definition itself.

The program will not behave differently if these recommendations are not followed, but the code will be less readable and it will be more likely that a change to the code will accidentally result in missing the initialization of a field.

If the types were more complex (i.e. class types with non-trivial constructors), then not initializing in a default member initializer or constructor initializer list would also be potentially problematic, since it would imply a default construction followed by assignment of the member instead of just direct proper construction.

Clang-tidy gives another important diagnostic for your code:

<source>:12:15: warning: field 'year' will be initialized after field 'name' [clang-diagnostic-reorder-ctor]
            : year(year), name(std::move(name)), surname(std::move(surname))
              ^~~~~~~~~~  ~~~~~~~~~~~~~~~~~~~~~  ~~~~~~~~~~~~~~~~~~~~~~~~~~~
              name(std::move(name)) surname(std::move(surname)) year(year)

You have the order of the initializers in the constructor wrong. When the class object is initialized, the members are always initialized in the order in which they were declared, not in the order in which they are listed in the constructor's initializer list. Therefore not replicating the exact same order can easily cause problems, for example when initialization of one member depends on the initialization of another member having completed and the constructor is written to make it look like that is the case, when it really isn't.

In the link above I have enabled all clang-tidy checks. Many of these will not really be important or won't even make sense for generic code at all, e.g. the llvmlibc-* checks you can see in the link. I can't say which of the checks are enabled in your build. It may be that only the first one but not the second one is enabled. If you didn't enable the checks yourself, you should ask whoever did, why certain checks should be considered relevant and others not. You can find the core guideline's recommendations via the links above. I think they are pretty widely accepted for the most part.

If you wanted to follow the core guidelines recommendations and still separate the calculation into a function, you should have the function return the value to initialize the member with:

class Employee
{
private:
    std::string name;
    std::string surname;
    int year;
    double salary;

    double calculateSalary() const
    {
        return 2310   2310 * year * 12 / 100.0;
    }

public:
    Employee(int year, std::string name, std::string surname)
            : name(std::move(name)), surname(std::move(surname)), year(year), salary(calculateSalary())
    {
    }
};

I also made the function private since it is only a helper function for the class. A user is not supposed to call it.