Thread: Iterator not working

Hello everyone. I recently changed from Visual Basic to Eclipse and I'm having a hard time getting used to this new IDE >_< (Nope, I didn't choose it).
Sometimes it detects unexistent errors which disappear after I restart Eclipse or when I coment/uncoment the code itself.
Therefore, I'm not sure whether the following piece of code is correct or not (even tho it considers it to be an error and I don't know why).

Code:

class Frota {
    vector<Vehicle *> vehicles;
public:
    int lowestYear() const;
};


int Frota::lowestYear() const{


    int lowestYear = 2014;


    vector<Vehicle *>::iterator it;
    int year = (*it)->getYear(); //getYear is a method from another class  


    if (vehicles.size() != 0) {
        for (it = vehicles.begin(); it != vehicles.end(); ++it) { //The error lies on this line of code
            if (year < lowestYear) {
                lowestYear = ano;
            }
        }
        return lowestYear;
    }
   return 0;
}

Error: no match for 'operator=' (operand types are .....) etc etc.

There isn't much I can say about it. I've done several iterators before, I don't understand why would the for loop be incorrect.

Any input is welcome.

This doesn't look like your actual code.

Line 14: vector<Vehicule*> may be different from vector<Vehicle*>.
Line 15: "it" is uninitialised (undefined behaviour).
Line 19: vector<Vehicle*>::iterator cannot be assigned to vector<Vehicule*>::iterator.
Line 26: no return statement.

You should declare variables in the smallest possible scope to avoid such problems...

Code:

#include <vector>
class Vehicle
{
public:
    int getYear(){return 2015;}
};
class Frota {
    std::vector<Vehicle *> vehicles;
public:
    int lowestYear() const;
};
 
 
int Frota::lowestYear() const{
 
 
    if (vehicles.size() != 0) {
        int lowestYear = 2014;
 
        for (auto it = vehicles.begin(); it != vehicles.end(); ++it) { 
            //Only here accessing vector members using it makes sence

            int year = (*it)->getYear();
            if (year < lowestYear) {
                lowestYear = year;
            }
        }
        return lowestYear;
    }
    return 0;
};

As a afterthought though - it seems usage of std::min_element would be even more suitable here

Something along the lines:

Code:

#include <algorithm>    // std::min_element, std::max_element
#include <vector>
class Vehicle
{
public:
    int getYear(){return 2015;}
};

bool myfn(Vehicle* i, Vehicle* j) { return i->getYear()<j->getYear(); }

class Frota {
    std::vector<Vehicle *> vehicles;
public:
    int lowestYear() const;
};
 
 
int Frota::lowestYear() const{
 
 
    int lowestYear = 2014;
 
    if (vehicles.size() != 0) {
        auto it = std::min_element(vehicles.begin(),vehicles.begin(),myfn);
        int year = (*it)->getYear();
        return year;
    }
    return 0;
};

PS. I'm to lazy to test the code above with real vector filled with values...

If you are allowed to use C++11, you can also use lambdas:

Code:

if (vehicles.size() != 0)
{
    auto compare = [](Vehicle* lhs, Vehicle* rhs) -> bool
    {
        return lhs->getYear() < rhs->getYear();
    };

    auto it = std::min_element(vehicles.begin(), vehicles.begin(), compare);
    int year = (*it)->getYear();
    return year;
}

The advantage is that you don't pollute the enclosing namespace with such a detail.

Originally Posted by Khabz

Although I don't understand why'd the compiler detect an error without "const".

Because if Frota::lowestYear() is a const-qualified member function, then all member variables of *this (including "vehicles") are seen const-qualified as well. If you were able to use vector<Vehicle*>::iterator inside Frota::lowestYear() const then you could accidentally modify "vehicles", which would break the constness of "Frota". That was the cause, but apparently noone here has noticed it before.