Like Tree2Likes
  • 1 Post By kmdv
  • 1 Post By tabstop

Iterator not working

This is a discussion on Iterator not working within the C++ Programming forums, part of the General Programming Boards category; Hello everyone. I recently changed from Visual Basic to Eclipse and I'm having a hard time getting used to this ...

  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 07:39 AM.

  2. #2
    Registered User
    Join Date
    Aug 2010
    Location
    Poland
    Posts
    682
    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.
    I never put signature, but I decided to make an exception.

  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
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    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;
    };
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  7. #7
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    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...
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  8. #8
    Registered User
    Join Date
    Aug 2010
    Location
    Poland
    Posts
    682
    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 09:22 AM.
    I never put signature, but I decided to make an exception.

  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
    682
    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.
    Khabz likes this.
    I never put signature, but I decided to make an exception.

  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.
    Khabz likes this.

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

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21