18

With the following code (excerpted for brevity):

color.h:

class color {
public:
    color();

    enum colorType {
        black, blue, green, cyan, red,
        magenta, brown, lightgray, nocolor
    };

    colorType getColorType();
    void setColorType(colorType cColortype);

    string getColorText() const;

private:
    colorType cColortype = nocolor;
    map<int, string> colors = {
        {black, "black"},
        {blue, "blue"},
        {green, "green"},
        {cyan, "cyan"},
        {red, "red"},
        {magenta, "magenta"},
        {brown, "brown"},
        {lightgray, "lightgray"},
        {nocolor, "nocolor"}};
};

color.cpp:

color::color() {
}

color::colorType color::getColorType() {
    return cColortype;
}

void color::setColorType(colorType cColortype) {
    this->cColortype = cColortype;
}

string color::getColorText() const {
    return colors[cColortype];
}

I get the following error:

color.cpp:16:29: error: passing 'const std::map >' as 'this' argument of 'std::map<_Key, _Tp, _Compare, _Alloc>::mapped_type& std::map<_Key, _Tp, _Compare, _Alloc>::operator[](std::map<_Key, _Tp, _Compare, _Alloc>::key_type&&) [with _Key = int; _Tp = std::basic_string; _Compare = std::less; _Alloc = std::allocator > >; std::map<_Key, _Tp, _Compare, _Alloc>::mapped_type = std::basic_string; std::map<_Key, _Tp, _Compare, _Alloc>::key_type = int]' discards qualifiers [-fpermissive]

The error refers to "return colors[cColortype];" in getColorText.

I'm writing this for a class project and I can get it to work for the sake of the assignment by removing the const declaration in the getColorText signature but I'm trying to learn/adopt good practices and adhere to the recommendation to use const for member functions that don't modify data so I want to know how to deal with this going forward.

I'm usually really good at debugging/troubleshooting but the error message is so convoluted that it's not much help.

Any help is appreciated.

WXB13
  • 1,046
  • 3
  • 11
  • 17
  • My first suggestion would be to remove the subscript operator in color::getColorText() const with a std::map<>::find call appropriately. – nakiya Nov 29 '13 at 03:04

3 Answers3

45
string color::getColorText() const {
    return colors[cColortype];
}

The issue is that you've marked the function as const. The operator[] on std::map is marked as non-const, and cannot be used in a const function like this. You need to manually use std::map::find (or other mechanism) to search for the input type and handle the case where it's not found.

If you're using C++11, you can instead use std::map::at, which IS allowed to be used on a constant map, and throws an exception if the requested element is not present.

Dave S
  • 20,507
  • 3
  • 48
  • 68
5

The key is near the end: "discards qualifiers". getColorText is a const member function, so colors is const. But map::operator[]() is not const.

Pete Becker
  • 74,985
  • 8
  • 76
  • 165
2

First : the map map<int, string> colors must be a map from cColorType to string instead of int :

map<cColorType, string> colors

Second : As some people already answered : map::operator[]() is not const. The reason for that is that this operator returns a reference, which allows you to modify its value.

Let me suggest the following solution : You can create a second private attribute : the color in string format. Therefore you'll have 2 Get functions (one for each type of color), and one Set function (which will modify the 2 color attributes).

Brainless
  • 1,522
  • 1
  • 16
  • 30
  • Actually cColorType is a variable name and not a type. Perhaps you meant colorType instead. This is actually not necessary as colorType is an enum which maps to int so the "map colors" signature works just fine. Part of the code that I left out of my post is an accessor that takes a string as an argument and performs a reverse map lookup on colors to obtain the corresponding colorType. The map with reverse lookup eliminates the need to store the color setting in 2 different formats. – WXB13 Nov 29 '13 at 05:33
  • Yes you're right about the cColorType, my apology. Keeping the first entry of map as "int" will work, but setting it to colorType is more flexible : imagine you (or someone else) will rewrite this code in the future, and would like something else than an enum for colorType. This coder won't have to change the declaration of the map. How do you perform the "reverse map lookup" ? I'm eager to see :) – Brainless Nov 29 '13 at 07:41
  • Reverse map lookup: http://stackoverflow.com/questions/5749073/reverse-map-lookup/19829556#19829556 – WXB13 Nov 29 '13 at 11:22
  • Your map reverse lookup uses **find_if**, which has linear complexity. If you store 2 different attributes, the 2 Get functions will execute in constant time each. And since the matching between the 2 types of color is perfect, you don't need reverse lookup. You can code instead two maps which represent inverse functions of each other. – Brainless Nov 30 '13 at 17:57