Second iterator overwrites first iterator

Question

I am not (yet) a good coder in C++ and I am working through some tasks from a training book, where I wrote a class for IP-addresses. In this class I have a private vector<string> which saves the single blocks of the address. I overloaded the == operator and here I hand over the object that I want to compare to by reference.

I have a vector::iterator for the this-object. I have now the problem that my second iterator that I defined for the compareTo-object overwrites the first iterator.

I have been searching for the mistake for hours. Please find the code snippet and its output attached. Any ideas? Or is this intended behavior of C++? I use CLANG-compiler on MacOS.

from Class IPv4

class IPv4
{
private:
string adress;
vector<string> IPBlocksOfAdress;

bool testIP();
vector<string> splitString(const string stringToSplit, char delimiter = '.');

public:
IPv4::IPv4(const string &inputAdress)
{
    setAdress(inputAdress);
}

bool IPv4::testIP()
{
    bool isIP = true;

    for (vector<string>::iterator blocksOfAdressIterator = IPBlocksOfAdress.begin();
     blocksOfAdressIterator != IPBlocksOfAdress.end(); blocksOfAdressIterator++) {

        if (stoi(*blocksOfAdressIterator) > 255 || stoi(*blocksOfAdressIterator) < 0) {
        isIP = false;
        }
    }
return isIP;
}

vector<string> IPv4::splitString(const string stringToSplit, char delimiter) 
{
    vector<string> blocksOfAdress;
    stringstream stringToSplitStream(stringToSplit);
    string blockOfAdress = "";
    while (getline(stringToSplitStream, blockOfAdress, delimiter)) {
        blocksOfAdress.push_back(blockOfAdress);
    }
    return blocksOfAdress;
}

string IPv4::getAdress() const
{
    if (adress != "0.0.0.0") {
        return adress;
    } else {
        return "Error, Adress not set.";
    }
}

vector<string> IPv4::getAdressBlocks() const
{
    return IPBlocksOfAdress;
}

void IPv4::setAdress(const string &inputAdress)
{
    IPBlocksOfAdress = splitString(inputAdress,'.');
    if (testIP()) {
        cout << "Neue IP Addresse: " << inputAdress << endl;
        adress = inputAdress;
    } else {
        cout << "Die Eingabe entspricht nicht dem IPv4-Format. Setze Adresse auf 0.0.0.0" << endl;
        adress = "0.0.0.0";
        IPBlocksOfAdress = splitString(adress,'.');
    }
}

bool IPv4::operator == (const IPv4 &compareTo) const
{
    cout << " ... in operator == ..." << endl;

    bool isIdenticalThis = true;

    cout << "this Adress: " << this->getAdress() << endl;
    cout << "compareTo Adress: " << compareTo.getAdress() << endl;

    vector<string>::iterator adressBlockThis = this->getAdressBlocks().begin();
    cout << "this first block: " << *adressBlockThis << endl;

    vector<string>::iterator adressBlockCompareTo = compareTo.getAdressBlocks().begin();
    cout << "compareTo first block: " << *adressBlockCompareTo << endl;
    cout << "this first block second printout: " << *adressBlockThis << endl;
    ...
}

from main:

IPv4 myIP("1.2.3.4");
cout << "Get IP: " << myIP.getAdress() << endl;

IPv4 mySecondIP("5.6.7.8");
cout << "Get IP: " << mySecondIP.getAdress() << endl;

cout << myIP.getAdress() << " == " << mySecondIP.getAdress() 
    << " = " << (myIP == mySecondIP) << endl;

output console:

Neue IP Addresse: 1.2.3.4
Get IP: 1.2.3.4
Neue IP Addresse: 5.6.7.8
Get IP: 5.6.7.8
1.2.3.4 == 5.6.7.8 =
 ... in operator == ...
this Adress: 1.2.3.4
compareTo Adress: 5.6.7.8
this first block: 1
compareTo first block: 5
this first block second printout: 5  <------- this should be 1 
1

Perhaps, but where is the code that makes the operation? Not int he question. — Matthieu Brucher, Nov 23 '18 at 21:28
I'm afraid you have left too many possibilities open. I recommend crafting a [mcve]. If making the MCVE doesn't reveal the problem and solution to you, and it usually does, edit the question to add the MCVE. — user4581301, Nov 23 '18 at 21:33
You are right, not enough information. So I attached the rest: — TheFox, Nov 23 '18 at 21:36
Still missing some: specifically, the implementation of `getAddressBlocks` — Sebastian Redl, Nov 23 '18 at 21:41
Sorry, formatting seems to be kind of complicated. It takes some time to copy from *.h and *.cpp — TheFox, Nov 23 '18 at 21:47
If you take the Tour and read the Help Centre, you'll find out how to format your post properly. — Lightness Races in Orbit, Nov 23 '18 at 22:40
I did. But the post struggled with a special character and did not show the rest from that character on. Next time I will focus on checking wether the editor takes my input correctly or not. — TheFox, Nov 24 '18 at 08:42
Because you wrote `<` but forgot to write the `>`, and didn't put it in code formatting tags as the Help Centre teaches. — Lightness Races in Orbit, Nov 24 '18 at 14:01

Lightness Races in Orbit · Accepted Answer · 2018-11-23T22:40:57.093

Your iterator isn't being "overwritten", it's being invalidated. It's dangling. Both of them are.

The numbers you see are pure chance. (Well, they aren't; we can reason about why you're seeing them to some degree, and in this case it's pretty obvious where they come from, but undefined behaviour is undefined behaviour and it's best to just leave it at that.)

getAdressBlocks should return a reference to the existing vector. The value/copy it's returning now dies pretty much immediately, leaving you with an iterator pointing to the first element of something that no longer exists.

You're calling this function through a const IPv4& so it will have to (and should) return a const reference:

const vector<string>& IPv4::getAdressBlocks() const
{
    return IPBlocksOfAdress;
}

This also means though that your iterators will have to be vector<string>::const_iterator.

And you've misspelt "address" throughout.

For the "Address" thing. I mixed up German and English. In German it is Adresse in Englisch it is address. — TheFox, Nov 24 '18 at 08:34

Second iterator overwrites first iterator

1 Answers1