CODEWITHSUNDEEP
c++
Rrobinvip
Rrobinvip

Reputation: 119

How could I iterator a vector as a member of class while the corresponding objects pass as a const

I have a simple class with only one member which is a vector, just like this:

class MyClass{
private:
    vector<double> vector_member;
public:
    method_1();
    method_2();
    ....
};

And when I trying to overload an operator such as +=, which should add each element in the vectors in two objects and return a vector as a sum of them. My approach so far is:

MyClass& MyClass :: operator+=(const MyClass& p){
    size_t n = max(this->vector_member.size(),p.vector_member.size());
    MyClass temp;
    for (size_t i = 0; i < n; ++i){
        temp.vector_member.push_back(0);
    }
    vector<double>::iterator it_1 = this->vector_member.begin();
    vector<const double>::iterator it_2 = p.vector_member.begin();
    vector<double>::iterator it_3 = temp.vector_member.begin();

    for (; it_1 != this->vector_member.end(); ++it_1, ++it_3){
        *it_3 += *it_1;
    }

    it_3 = temp.vector_member.begin();

    for (; it_2 != p.vector_member.end(); ++it_2, ++it_3){
        *it_3 += *it_2;
    }

    return temp;
}

The reason that I set up a temp vector is, I need a vector with the max size of those two vectors to avoid Segmentation fault.

My problem is at each time when I try to do vector<const double>::iterator it_2 = p.vector_member.begin(); or it_2 != p.vector_member.end().

The vs code seems not happy with this. It says cannot convert between "__gnu_cxx::__normal_iterator<const double *, std::vector<double, std::allocator<double>>>" and "__gnu_cxx::__normal_iterator<const double *, std::vector<const double, std::allocator<const double>>>"

I don't know how to fix this, and also is there any smart way to do this? I think my code is so broken. Thanks

Upvotes: 3

Views: 115

Answers (2)

Remy Lebeau
Remy Lebeau

Reputation: 595827

The input MyClass p object is const, so its vector<double> is also const, and as such you need to use vector<double>::const_iterator to iterate its elements, not vector<const double>::iterator. That is why you are getting the compiler error.

But, even fixing that, your code is still wrong. operator+= is supposed to modify this and then return *this. But you are not doing that. You are creating a whole new MyClass object and returning a (dangling) reference to that instead. Your operator+= is acting more like how operator+ should act (see What are the basic rules and idioms for operator overloading?).

Try this instead:

MyClass& MyClass :: operator+=(const MyClass& p){
    size_t n = p.vector_member.size();
    if (n > this->vector_member.size())
        this->vector_member.resize(n);

    vector<double>::iterator it_1 = this->vector_member.begin();
    vector<double>::const_iterator it_2 = p.vector_member.begin();

    for (size_t i = 0; i < n; ++i){
        *it_1++ += *it_2++;
    }

    /* alternatively:

    size_t n = std::max(this->vector_member.size(), p.vector_member.size());
    std::vector<double> temp(n, 0.0);

    vector<double>::iterator it_1 = this->vector_member.begin();
    vector<double>::const_iterator it_2 = p.vector_member.begin();
    vector<double>::iterator it_3 = it_3 = temp.begin();

    while (it_1 != this->vector_member.end()){
        *it_3++ += *it_1++;
    }

    it_3 = temp.begin(); 
    while (it_2 != p.vector_member.end()){
        *it_3++ += *it_2++;
    }

    this->vector_member = std::move(temp);
    */

    return *this;
}

MyClass operator+(const MyClass& lhs, const MyClass& rhs){
    MyClass temp(lhs);
    temp += rhs;
    return temp;
}

Upvotes: 1

Sam Varshavchik
Sam Varshavchik

Reputation: 118310

vector<const double>::iterator it_2 = p.vector_member.begin();

This should be:

vector<double>::const_iterator it_2 = p.vector_member.begin();

Your vector is always a vector of double values. It is the vector itself that's constant here. It does make the contents of the vector effectively const, but they are still doubles, "by the book".

Also, in the post C++11 world, this can be simply:

auto it_2 = p.vector_member.begin();

And let the chips fall where they may...

P.S. The operator you implemented is fundamentally broken for a reason unrelated to your compilation error: it's returning a reference to a temporary object. You really implemented an operator+, if you return not a reference but the temp value itself, and not a reference to it. An operator+= is expected to modify itself, rather than create a new object and return it. In any case, there's appears to be a much simpler and shorter way to accomplish a logically equivalent operator+: copy-construct your temp from *this, resize its vector to the same max() value (which will zero-initialize any new values automatically), then simply add the other vector's values to the copy's vector. Should be about half as much code.

Upvotes: 4

Related Questions