Why is my overloaded subscript operator for rvalue not called

Viewed 127

I have looked up and down stack overflow and keep finding the same examples which I think I have implemented. I am trying to implement an associative array. I know there is std::map but I'd like to do the implementation myself for better control and better understanding.

I have overloaded the subscript operator for lvalue and rvalue. However in my code only the method for the lvalue is called and I can't find where I am mistaken. Can anyone point me in the right direction, please? Here's my code for the class. For now, it's not supposed to be efficient, just working would make me happy:

template<typename K, typename V>
class AssocArray {

private:
    size_t _arraySize = 0;
    K *_keyArray = nullptr;
    V *_valueArray = nullptr;

    void expandValueArray() {
        auto newValueArray = new V[_arraySize];
        // copy old values
        for (size_t i = 0; i < _arraySize - 1; ++i)
            newValueArray[i] = _valueArray[i];
        if (_valueArray)
            delete[] _valueArray;
        _valueArray = newValueArray;
    }

    void appendToKeyArray(K key) {
        auto newKeyArray = new K[_arraySize];
        // copy old keys
        for (size_t i = 0; i < _arraySize - 1; ++i)
            newKeyArray[i] = _keyArray[i];
        newKeyArray[_arraySize - 1] = key;
        delete[] _keyArray;
        _keyArray = newKeyArray;
    }

    bool keyExists(K key) {
        for (size_t i = 0; i < _arraySize; ++i)
            if (_keyArray[i] == key)
                return true;
        return false;
    }

    size_t getExistingKeyIndex(K key) {
        for (size_t i = 0; i < _arraySize; ++i)
            if (_keyArray[i] == key)
                return i;
    }

public:
    ~AssocArray() {
        delete[] _valueArray;
        delete[] _keyArray;
    }

    V operator[](K key) const {
        if (keyExists(key))
            return _valueArray[getExistingKeyIndex(key)];
        else
            throw std::out_of_range("Key does not exist");
    }

    V &operator[](K key) {
        if (keyExists(key))
            return _valueArray[getExistingKeyIndex(key)];

        // key does not exist
        ++_arraySize;
        appendToKeyArray(key);
        expandValueArray();
        return _valueArray[_arraySize - 1];
    }

    void print() {
        std::cout << "Content of AssocArray:" << std::endl;
        if (!_arraySize) std::cout << "none" << std::endl;
        for (int i = 0; i < _arraySize; ++i) {
            std::cout << "[" << _keyArray[i] << "] => " << _valueArray[i] << std::endl;
        }
    }
};

and here is how I call it and produce the undesired behavior:

#include <iostream>
#include "AssocArray.h"

int main() {
       AssocArray<std::string, std::string> assocArray;

        (assocArray)["Toni"] = "seven";
        (assocArray)["Sam"] = "five";
        std::cout << "assocArray before lookup of non existing key:" << std::endl << std::endl;
        assocArray.print();
        // FixMe: appends key to assocArray, but shouldn't
        std::cout << std::endl << "lookup of non existing key:" << std::endl;
        auto key = "Megan";
        std::cout << "[" << key << "] => " << assocArray[key] << std::endl << std::endl;

        std::cout << "assocArray after lookup of non existing key:" << std::endl;
        assocArray.print();
}

The output is this (see that Megan is added to the array where it shouldn't be):

assocArray before lookup of non existing key:

Content of AssocArray:
[Toni] => seven
[Sam] => five

lookup of non existing key:
[Megan] => 

assocArray after lookup of non existing key:
Content of AssocArray:
[Toni] => seven
[Sam] => five
[Megan] => 

Why don't I get an exception? Why is the overloaded method for the lvalue called here? Thanks for looking into this.

1 Answers

The issue in question

Turns out, you're asking the wrong question. You're having a logic problem, which in this particular case has nothing to do with ref-qualification of the member function. In your code, I can see no trace of any use where the AssocArray is being an rvalue (on which we want to call operator[]).

To have a viable solution, I would suggest you to look toward two following approaches:

  1. Recall, that when we subscript std::map and there's no such key yet, the std::map will add such key in with default value. This is exactly what's happening in your current implemenation. To not insert an element every time we want to fetch one, we have at(). So, you can just add equivalent at() member function, which may throw or just silently return some default value, denoting there's no such element.
  2. You can throw on every invalid fetch. If the subscript is done on a valid key, then return a & to the according mapped_type (the value mapped to the key), which you can read/modify.

Ref-qualifiers

You should consider the difference:

V& operator[]();
V operator[]() const;

Vs.

// An example approach to return types
// You should look mainly on the ref-qualifiers here
V& operator[]() &&; // called on rvalues
const V& operator[]() const&; // called on const and non-const lvalues

In your class, you have one [] that returns a copy of a fetched element, and another [] that is called on a const instance and returns a reference. There are no operators which distinct whether they are called on an rvalue or lvalue.

For more information on ref-qualifiers, you may refer to the documentation.

Related