CODEWITHSUNDEEP
c++c++11
R Sheath
R Sheath

Reputation: 91

When should const_iterator be used instead of auto

Below is an example that I think illustrates a case where it is preferable to use const_iterator to "const auto". This is because containers don't provide a cfind() function. Is there any alternative? Or should "const auto" be used and the lack of const be ignored?

std::string GetJobTitle(const std::string& employee)
{
    using EmployeeTitles = std::unordered_map<std::string, std::string>;
    EmployeeTitles employees = { { "Alice", "Director" },{ "Bob","Manager" } ,{ "Charlie", "Developer" } };

    // Option 1. const_iterator is access only:
    EmployeeTitles::const_iterator itr = employees.(employee);
    if (itr != employees.cend())
    {
        itr->second = "Project Manager"; // error C2678: The compiler prevents us changing the job tile which is what we want
        return itr->second;
    }

    // Option 2. const auto is more concise but is not as safe:
    const auto& itr2 = employees.find(employee);
    if (itr2 != employees.cend())
    {
        itr2->second = "Project Manager"; // employee now has new title - how can we prevent this with the compiler but still use auto?
        return itr2->second;
    }
    return "";
}

Upvotes: 6

Views: 1236

Answers (1)

besc
besc

Reputation: 2647

If possible sidestep the problem

Use const variables

Your example doesn’t illustrate a good case for the problem. Just be more “aggressive” with constness and it disappears. You do not change employees at all, so the proper solution is to declare it const in the first place:

const EmployeeTitles employees = ...;

That’s even safer because it prevents changes to employees anywhere, not just changes through the iterator.

Use scoping to separate const/non-const code

What if you cannot make employees const because you can only populate it piece by piece; for example because you pull the information from a database? Move the populating code into a builder function. Or for simple cases use an immediately invoked lambda:

const EmployeeTitles employees = [] {
    EmployeeTitles employees;

    for (const auto& record : database.employees()) {
        // Probably some more processing would be done here in the real world.
        employees.emplace(record.name(), record.job_title());
    }

    return employees;
}();

Use const member functions

If employees is a member variable of a class and you iterate over it in a member function, make that function const.

As a general rule

Whenever you run into this problem or something similar, think of ways to use const variables/functions and/or scoping to sidestep it altogether. That will take care of the majority of cases.

What if you do run into it?

In that case I’d go with your option 1: Explicitely declaring the iterator const_iterator in combination with the using declaration for the map type. It’s concise, readable, immediately understandable, and the most direct way to express your intent.

Other solutions that manipulate the constness of employees aren’t as good because that’s not what you really care about. What you actually want is a read-only iterator. Fiddling with the constness of employees is just a roundabout way to reach that goal. And roundabout code is harder to understand.

On the other hand that doesn’t mean you’ll run into huge problems with clarity. Especially std::as_const is nice and concise as well.

However with a pre-C++17 code base you’d have to use a const_cast. It’s a benign one because it adds const and it’s not too verbose, either. But I’d avoid it on the general principle that seeing a const_cast in a piece of code is always a bit scary on first glance. Another good possibility, as @Swift pointed out in the comments, is to implement your own version of as_const.

Upvotes: 5

Related Questions