0

I am trying to overload the subscript operator [] in my class which uses a linked list to create a map. This and several variations, like adding const, is what I have tried.

header

int& operator[](std::string key);

and then defining the overload in a seperate file

int& mapLL::operator[](std::string key){ 
   int val = this->get(key);
   return val;
}

this is the error I don't know how to fix

main.cpp: In function ‘int main()’:
main.cpp:38:24: error: invalid types ‘mapLL*[const char [4]]’ for array subscript
 int a = list3["ghi"]; 
                    ^
mapLL.cpp: In member function ‘int& mapLL::operator[](std::string)’:
mapLL.cpp:110:9: warning: reference to local variable ‘val’ returned [-Wreturn-local-addr]
   int val = this->get(key);
       ^

Then in the main file I am trying this

mapLL *list3 = new mapLL();
list3->set("abc",1);
list3->set("def",2);
list3->set("ghi",3);
list3->set("jkl",1);
list3->toString();
cout << list3->get("ghi") << endl;
int a = list3["ghi"]; 
cout << a << endl;
delete list3;

get function

int mapLL::get(std::string key){
    bool found = false;
    node *me = (node *) first;
    if(is_empty()){
        return -2;
    }
    while(!found){
        if (me->getKey() == key){
            return me->getValue();
        }else{
            if (me->getNext() == 0){
                return -1;
            }else{
                me = (node *) me->getNext();
            }
        }
    }
}
cmb
  • 95
  • 3
  • 12
  • It looks like `list3` is a pointer, so you would have to do `(*list3)["ghi"]` – Brian Bi Oct 15 '15 at 19:07
  • Also, as the compiler is warning you, the method returns a reference to a temporary. This is definitely not what you want to be doing. – Jon Oct 15 '15 at 19:10
  • Is there a reason you need to use dynamic memory allocation (e.g. operator new)? This is not Java or C#. You can declare `list3` as a local or global variable without `new`. – Thomas Matthews Oct 15 '15 at 19:10
  • @Brian I just tried your suggestions and that worked is there a way I can rewrite the function so `list3["ghi"]` works – cmb Oct 15 '15 at 19:11
  • @cmb don't declare `list3` as a pointer. It doesn't need to be one. – jaggedSpire Oct 15 '15 at 19:13
  • @cmb The issue is not your function. You declared list3 as a pointer to mapLL so naturally you need to dereference it to get at the underlying object. Try declaring list3 like this `mapLL list3;` Then you could get `list3["ghi"]` to work. – Mohamad Elghawi Oct 15 '15 at 19:14
  • Can you post get() function? – Cem Kalyoncu Oct 15 '15 at 19:22

2 Answers2

4
int& mapLL::operator[](std::string key){ 
   int val = this->get(key);
   return val;
}

you are returning a reference to a local variable, val.
what you actually need to do is to find the element in you linked list and return it as is, no assignment to local variables in between.

Plus, list3 is a pointer, unfortunatly, you need to dereference it before using [] operator :

(*list3)["ghi"]; 

all have being said + looking at your profile, I get that you come from a Java background. my advice - understand what is the difference between stack allocation and heap allocation. this is the very basic of the language. you need to use dynamically allocated objects (=using new) very rarely.

although Java hides away allocation details, this is maybe the one of the most important subjects in C++. not everything has to be a pointer. your starting point is stack allocated objects. move from there to dynamic /static allocation if it does not line with your needs.

David Haim
  • 25,446
  • 3
  • 44
  • 78
  • my origonal idea was this `return this->get(key)` but it gave me this error `invalid initialization of non-const reference of type ‘int&’ from an rvalue of type ‘int’` – cmb Oct 15 '15 at 19:14
1

I recommend to refrain from using raw pointers and dynamic allocation. Your issue stems from incorrect use of pointers.

Use direct declarations:

mapLL list3;
list3.set("abc",1);
list3.set("def",2);
list3.set("ghi",3);
list3.set("jkl",1);
list3.toString();
cout << list3.get("ghi") << endl;
int a = list3["ghi"]; 
cout << a << endl;
Thomas Matthews
  • 56,849
  • 17
  • 98
  • 154