CODEWITHSUNDEEP
c++classiterator
Jallal
Jallal

Reputation: 101

C++ Iterator class

class ZoningIter
{
private:
    CTile::Zonings mZoning;
    CCity *mCity;
    int mPos;       ///< Position in the collection

public:
    ZoningIter(CCity *tile, int pos, CTile::Zonings zoning) : mCity(tile), mPos(pos), mZoning(zoning) 
    {       
        while (mPos < mCity->mTiles.size())
        {
            if (mCity->mTiles[mPos]->GetZoning() == mZoning)
                break;
            else
                mPos++;
        }       
    }

    bool operator!=(const ZoningIter &other) const
    {
        return mPos != other.mPos;
    }

    std::shared_ptr<CTile> operator *() const
    {               
        return mCity->mTiles[mPos]; 
    }

    const ZoningIter& operator++()
    {                       
        auto size = mCity->mTiles.size();
        auto myzone = mCity->mTiles[mPos]->GetZoning();

        while (mPos < size-1)
        {
            ++mPos;
            if (mCity->mTiles[mPos]->GetZoning() == mZoning)
                break;
        }   
        return *this;           
    }
};

Not sure what am I doing wrong in this iterator. This class Iterator is suppose to loop over the class CCity and compare the tiles coming back with the ones that I am looking for. one issue that I can see it run into an infinite loop not sure how to fix it ? any ideas?

Upvotes: 0

Views: 234

Answers (1)

Deduplicator
Deduplicator

Reputation: 45694

You are running into an infinite loop because your operator++ won't increment to one past all elements, also known as an end-iterator.

while (mPos < size-1) // <-- This line is wrong, stops too early

It masks the error in the check-for-success you do afterwards.

Anyway, it should start with an increment, and then check whether it can and should loop.
A corrected version, with both bugs removed:

const ZoningIter& operator++() {
    while( ++mPos < mCity->mTiles.size()
        && mCity->mTiles[mPos]->GetZoning() != mZoning)
        {}
    return *this;
}

Also, the return-type of operator*() should be const std::shared_ptr<CTile>&, the caller can make a copy if needed.

Upvotes: 1

Related Questions