Reputation: 83

operator[]= overload?

Okay, I'm trying to make a quick little class to work as a sort of hash table. If I can get it to work then I should be able to do this:

  StringHash* hash = new StringHash;
  hash["test"] = "This is a test";
  printf(hash["test"]);

And it should print out "This is a test".

It looks like I have 2 problems at this point. Firstly I did this:

const char* operator[](const char* key) {
  for(int i = 0; i < hashSize; ++i) {
    if(strcmp(hkeys[i], key) == 0) {return values[i];}
  }
  return NULL;
}

But when I try to look up a value the compiler complains that

error: invalid types `StringHash*[const char[5]]' for array subscript

Secondly operator[]= does not appear to be the correct syntax here. The only other thing I could find was &operator[] but I don't think that will work since I have to code the lookup procedure??? (Isn't that syntax just used to return an array item reference?)

Is what I'm trying to do here even possible? Any advice appreciated. :)

Seems to be some confusion about what I'm trying to do. I'll post my code:

http://pastebin.com/5Na1Xvaz

Finished product after all the help:

http://pastebin.com/gx4gnYy8

Upvotes: 8

Answers (8)

CashCow

Reputation: 31435

I would firstly question why you are writing your own HashMap when there are some versions available albeit not a standard one. (that was written in 2010, but there is now std::unordered_map)

Does your hash-map store const char* pointers or std::strings? (It might store const char * pointers if it is simply a lookup table to data stored elsewhere that is not going to change its lifetime).

What is operator[] supposed to do when the item is not found?

Now let me assume that the answers are:

Yes we are storing const char * pointers, and we store a NULL in an empty cell
When we perform hash[key]=value we want to associate key with value
If we just do hash[key] but don't write, it does not insert

This can be done with a magic object: when you assign a const char * to this object it inserts or overwrites to the hash. You can also have an implicit conversion from the object to const char * for reading.

This is quite complex though and it is preferable to stick to the regular interface of map: operator[] always inserts and you use a different method just to lookup.

Upvotes: 3

Platinum Azure

Reputation: 46183

The other answers relate to your first question. As for your second...

If you return a reference, then you're returning an lvalue. You can always assign to an lvalue.

Yes, it (pretty much) really is that simple. I recommend reading carefully for whether or not you need const in various places, though.

What I remember reading is that you should provide a const and a non-const overload for operator[], something like so:

MyType const &operator[](int index) const; // This is the array access version (no assignment allowed), which should work on const objects
MyType &operator[](int index);      // This is the array access or assignment version, which is necessarily non-const.

See this link for more information.

Upvotes: 6

Cheers and hth. - Alf

Reputation: 145214

Write StringHash hash; instead of the new thing. C++ isn't Java. :-)

Upvotes: 3

Mark B

Reputation: 96233

Are you able to use a boost::unordered_map<std::string, std::string? Then you don't have to worry about implementing this on your own.

Assuming it's an exercise of some sort for yourself: You may come from different background, but in C++ the normal way to declare your hash would be:

StringHash hash;

Additionally your operator[] might work for the print but it won't work for the assignment. Normally operator[] methods work by returning a non-const reference or a proxy object to which the new values can be assigned, and yours does neither. If you were able to use std::string, you could rewrite your method to return a non-const reference to the position within the hash that should be read or assigned.

Upvotes: 1

Steve M

Reputation: 8516

Five problems:

hash is a pointer to a StringHash, you have to dereference it to use operators: (*hash)["test"]
If you want to assign to the element, you have to return a reference to the element type

const char *& operator[] (const char* key);

// ...

(*hash)["test"] = "This is a test"; // will compile now
null isn't a keyword in C++. Use 0 or NULL.
operator [] has to allocate space for an element if it isn't found. Returning NULL isn't an option. Otherwise, trying to assign to the result of (*hash)["test"] will crash your program.
Use std::map or std::tr1::unordered_map instead of writing your own "quick" class.

And just to be a jerk: You know that isn't a hash table, right?

Upvotes: 3

sellibitze

Reputation: 28087

The first error is that you declared hash to be a pointer. Pointer types can already be used with the index operator. For example, pointer[3] is equivalent to *(pointer+3). You can't change that behaviour. Make hash the object itself:

StringHash sh;

As for operator[]=, there is no such thing. Your index operator should just return a reference in order to make the assignment work. Here's a simple example of how this would look like:

class Indexable
{
   std::string arr[3];
public:
   std::string & operator[](int index) {
      return arr[index];
   }
   std::string const& operator[](int index) const {
      return arr[index];
   }
};

Upvotes: 3

John Dibling

Reputation: 101456

hash isn't a StringHash object. Its a pointer to one.

You can do this:

(*hash)["test"] = "This is a test";

Or you can ask yourself why you need a pointer to it in the first place,

StringHash hash;
hash["test" = "This is a test";

... or even if you do, why you wouldn't use a smart pointer like auto_ptr.

#include <memory>
std::auto_ptr<StringHash> hash( new StringHash );
(*hash)["test"] = "This is a test";

Upvotes: 3

Yakov Galka

Reputation: 72469

The error is because hash is a pointer. Change to:

StringHash hash;

Upvotes: 7

operator[]= overload?

Answers (8)

Related Questions