Thread: Iterator not working

  1. #1
    Registered User
    Join Date
    Mar 2013
    Location
    Portugal, Porto.
    Posts
    105

    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.
    Last edited by Khabz; 10-18-2013 at 06:39 AM.

  2. #2
    Registered User
    Join Date
    Aug 2010
    Location
    Poland
    Posts
    733
    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.

  3. #3
    Registered User
    Join Date
    Mar 2013
    Location
    Portugal, Porto.
    Posts
    105
    I traduced a few keywords to English so you would understand it better and added a return statement in the end. Fixed now. And yes, it is my code.

    Also, what do you mean? "Line 15: "it" is uninitialised (undefined behaviour)." I thought this piece of code
    Code:
     vector<Vehicle *>::iterator it; 
    was considered initialization.

  4. #4
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    And surely your year variable has to be inside your for-loop and not above it?

  5. #5
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Quote Originally Posted by Khabz View Post
    I traduced a few keywords to English so you would understand it better and added a return statement in the end. Fixed now. And yes, it is my code.

    Also, what do you mean? "Line 15: "it" is uninitialised (undefined behaviour)." I thought this piece of code
    Code:
     vector<Vehicle *>::iterator it; 
    was considered initialization.
    That just declares a variable, in this case an uninitialized variable. If you had an initialization portion there, like
    Code:
    vector<Vehicle *>::iterator it = something;
    then you'd have an initialized variable. (But see my other post for why this line is wrong anyway.)

  6. #6
    Hurry Slowly vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,794
    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;
    };
    All problems in computer science can be solved by another level of indirection,
    except for the problem of too many layers of indirection.
    David J. Wheeler

  7. #7
    Hurry Slowly vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,794
    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...
    All problems in computer science can be solved by another level of indirection,
    except for the problem of too many layers of indirection.
    David J. Wheeler

  8. #8
    Registered User
    Join Date
    Aug 2010
    Location
    Poland
    Posts
    733
    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.
    Last edited by kmdv; 10-18-2013 at 08:22 AM.

  9. #9
    Registered User
    Join Date
    Mar 2013
    Location
    Portugal, Porto.
    Posts
    105
    I really appreciate your attempts at finding a new and more suitable solution but I rather have my method working.

    After some research I came up with a solution: I switched from
    Code:
     vector<Veiculo *>::iterator it;
    to
    Code:
     vector<Veiculo *>::const_iterator it;
    It does make sense considering the vector "vehicles" is not changed at all.
    Although I don't understand why'd the compiler detect an error without "const".

    Here's how it looks now.

    Code:
    class Frota {
        vector<Vehicle *> vehicles;
    public:
        int lowestYear() const;
    };
    
    
    int Frota::lowestYear() const{
    
    
        int lowestYear = 2014;
     
    
    
        vector<Vehicle *>::const_iterator it;
    
     
        if (vehicles.size() != 0) {
            for (it = vehicles.begin(); it != vehicles.end(); ++it)
                int year = (*it)->getYear();
                if (year < lowestYear) {
                    lowestYear = ano;
                }
            }
            return lowestYear;
        }
       return 0;
    }

  10. #10
    Registered User
    Join Date
    Aug 2010
    Location
    Poland
    Posts
    733
    Quote Originally Posted by Khabz View Post
    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.

  11. #11
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Quote Originally Posted by Khabz View Post
    Although I don't understand why'd the compiler detect an error without "const".
    I'm going to guess that's because you have the function marked as a const function, so it has to be able to work on a const vector.

  12. #12
    Registered User
    Join Date
    Mar 2013
    Location
    Portugal, Porto.
    Posts
    105
    Quote Originally Posted by kmdv View Post
    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.
    I understand it now, thank you very much.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 9
    Last Post: 03-30-2009, 04:09 AM
  2. Connecting input iterator to output iterator
    By QuestionC in forum C++ Programming
    Replies: 2
    Last Post: 04-10-2007, 02:18 AM
  3. Do we really need iterator?
    By Antigloss in forum C++ Programming
    Replies: 3
    Last Post: 06-13-2005, 10:59 AM
  4. need help with std::set::iterator
    By WebmasterMattD in forum C++ Programming
    Replies: 1
    Last Post: 05-16-2003, 08:55 AM
  5. iterator
    By Luigi in forum C++ Programming
    Replies: 4
    Last Post: 12-18-2002, 01:43 PM