Operator overloading [] and =

Question

Im trying to build a template class that is a simplified version of C# dictionary class, called "Map". The problem I have is with [] operator overload and = operator overload.

[] operator overload should be overloaded in a way that allows statement like mapObject[“keyString”]=value; in such cases if the key does not exist an exception must be thrown. An overloaded = operator should perform a deep copy.

What am I doing wrong?

    #pragma once
#include <iostream>
#include <ostream>

using namespace std;

template<class K, class V>
class Map;

template <class K, class V>
class Entry;

//template<class K, class V>
//Map<K, V>& operator[](K* sKey);

template<class K, class V>
ostream& operator<< <>(ostream& os, const Map<K, V>& L);

template <class K, class V>
class Map
{
protected:
    Entry<K, V>     **ledger;
    Entry<K, V>     **temp;
    int             numOfEntries;

public:
    V               *valArray;

    Map();
    Map(const Map<K, V>& mapObject);
    ~Map();

    void ledgerAllocation();
    void tempAllocation();
    void entrySizePlus();
    void ledgerCopy();
    void tempCopy();
    void AddEntry(K key, V value);

    Map<K, V>& operator[](K* sKey);
    friend ostream& operator<< <>(ostream& os, const Map& L);
    Map<K, V>& operator=(const Map<K, V>& toBeCopied);
};


//Default constructor, might change at a later stage...
template<class K, class V>
Map<K, V>::Map()
{
    numOfEntries = 1;
}

template<class K, class V>
inline Map<K, V>::Map(const Map<K, V>& mapObject)
{
    ledger = new Entry<K, V>*;
    numOfEntries = 1;
    ledger = mapObject.ledger
}

//Default destructor, will need to change it later to properly delete an allocated array.
template<class K, class V>
Map<K, V>::~Map()
{
}

template<class K, class V>
inline void Map<K, V>::ledgerAllocation()
{
    ledger = new Entry<K, V>*[numOfEntries];

    for (int i = 0; i < numOfEntries; i++)
    {
        ledger[i] = new Entry<K, V>;
    }
}

template<class K, class V>
inline void Map<K, V>::tempAllocation()
{
    temp = new Entry<K, V>*[numOfEntries];

    for (int i = 0; i < numOfEntries; i++)
    {
        temp[i] = new Entry<K, V>;
    }
}

template<class K, class V>
inline void Map<K, V>::entrySizePlus()
{
    tempAllocation();
    ledgerCopy();

    ledgerAllocation();
    tempCopy();
}

template<class K, class V>
inline void Map<K, V>::ledgerCopy()
{
    tempAllocation();
    K cKey;
    V cValue;

    for (int i = 0; i < numOfEntries - 1; i++)
    {
        cKey = ledger[i]->getKey();
        cValue = ledger[i]->getValue();
        temp[i]->setKeyandValue(cKey, cValue);
    }
}

template<class K, class V>
inline void Map<K, V>::tempCopy()
{
    ledgerAllocation();
    K cKey;
    V cValue;

    for (int i = 0; i < numOfEntries - 1; i++)
    {
        cKey = temp[i]->getKey();
        cValue = temp[i]->getValue();
        ledger[i]->setKeyandValue(cKey, cValue);
    }
}

//Function that will add a new Key and Value to a new map-object within the array.
template<class K, class V>
void Map<K, V>::AddEntry(K newKey, V newValue)
{

    if (numOfEntries <= 1)
    {
        ledgerAllocation();
    }
    else
    {
        entrySizePlus();
    }

    this->ledger[numOfEntries - 1]->setKeyandValue(newKey, newValue);
    numOfEntries++;
}


template<class K, class V>
Map<K, V>& Map<K, V>::operator[](K * sKey)
{
    K Akey;
    V Avalue;

    for (int i = 0; i < numOfEntries; i++)
    {
        Akey = ledger[i]->getKey();
        Avalue = ledger[i]->getValue();

        if (Akey == *sKey)
        {
            return Avalue;
        }
        else
        {
            throw string("Key does not exist");
        }
    }
}

template<class K, class V>
Map<K, V>& Map<K, V>::operator=(const Map<K, V>& toBeCopied)
{
    if (ledger == toBeCopied)
    {
        return *this;
    }
    ledger = toBeCopied.ledger;

    return *this;
}

//Overloads the "<<" so that the command: cout << mapObject << endl; can be used to print the entire map.
template<class K, class V>
ostream & operator<< <>(ostream& os, const Map<K, V>& L)
{
    K pKey;
    V pValue;

    for (int i = 0; i < L.numOfEntries - 1; i++)
    {
        pKey = L.ledger[i]->getKey();
        pValue = L.ledger[i]->getValue();
        os << "[" << pKey << " , " << pValue << "]";
        if (i < L.numOfEntries - 2)
        {
            cout << " , ";
        }
    }
    return os;
}


template <class K, class V>
class Entry
{
protected:
    K key;
    V value;

public:
    Entry();
    ~Entry();

    void setKeyandValue(K skey, V sValue);
    K getKey();
    V getValue();
    void printKeyandValue();

};


template<class K, class V>
Entry<K, V>::Entry()
{
    this->key = NULL;
    this->value = NULL;
}

template<class K, class V>
Entry<K, V>::~Entry()
{
}

template<class K, class V>
inline void Entry<K, V>::setKeyandValue(K skey, V sValue)
{
    key = skey;
    value = sValue;
}

template<class K, class V>
inline K Entry<K, V>::getKey()
{
    return this->key;
}

template<class K, class V>
inline V Entry<K, V>::getValue()
{
    return this->value;
}

template<class K, class V>
void Entry<K, V>::printKeyandValue()
{
    cout << this->key << ": " << this->value << endl;
}

ok, sure. Then I would suggest to boil down the code to the actual problem. — davidhigh, Apr 03 '16 at 16:35
This _might_ be useful/interesting to you: [What is the copy-and-swap idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). — Jorn Vernee, Apr 03 '16 at 16:46

score 0 · Answer 1 · answered Apr 03 '16 at 16:35

0

    Akey = ledger[i]->getKey();
    Avalue = ledger[i]->getValue();

    if (Akey == *sKey)
    {
        return Avalue;

    // ...

What you're doing wrong here is that you're returning a reference to an object that goes out of scope when this function returns. As soon as this function returns, this reference is no longer valid.

    Avalue = ledger[i]->getValue();

Here, you're calling getValue(), and placing the result of this method call into an object that's a "local" object that's declared in this function.

    return Avalue;

And this returns a reference to this object. But because this is a local object to this function, as soon as this object is returned, the object no longer exists.

Your general approach is correct, your operator[] needs to return a reference.

However, you need to return a reference to the original value in your container, instead of a "local" object, a temporary object that's created in the operator[] method, and gets destroyed when this method returns.

answered Apr 03 '16 at 16:35

Sam Varshavchik

114,536
5
94
148

So i can still use Akey= ledger[i]->getKey(); to compare the keys? – alifarlig Apr 03 '16 at 16:42
Comparing the keys has nothing to do with correctly returning the value. – Sam Varshavchik Apr 03 '16 at 16:46
So I can remove V Avalue; Avalue = ledger[i]->getValue(); return Avalue; And replace it with something like return ledger[i].value; ? – alifarlig Apr 03 '16 at 16:56
When I test my operator overload, I get an error "no operator "[]" matches these operands. And C2679 "binary '[': no operator found which takes a right-hand operand of type 'int' (or there is no acceptable conversion). Why is that? – alifarlig Apr 03 '16 at 18:18
Well, because there's something wrong with it, of course. You should either amend this question, or post a new [mcve]. – Sam Varshavchik Apr 03 '16 at 18:27

score 0 · Answer 2 · answered Apr 03 '16 at 16:37

0

An overloaded = operator should perform a deep copy.

In Map<K, V>& Map<K, V>::operator=(const Map<K, V>& toBeCopied) The line: ledger = toBeCopied.ledger; does not perform a deep copy. Both objects will point to the same ledger.

answered Apr 03 '16 at 16:37

Jorn Vernee

31,735
4
76
93

Operator overloading [] and =

2 Answers2