CODEWITHSUNDEEP
c++vector
Dementor
Dementor

Reputation: 251

C++ vector accessing elements

How can I access elements from myVector like i would do with arrays ( for(i = 0; i < n; i++) cout << v[i] << " "; )

My code:

#include <iostream>
#include <vector>
#include <string>
using namespace std;

class Month
{
public:
    char *name;
    int nr_days;
    Month(char* c, int nr) : name(c), nr_days(nr){};
    ~Month() { /* free(name); */}
};

int main()
{
    Month January("January", 31);
    Month February("February", 28);
    Month March("March", 31);
    Month April("April", 30);
    Month May("May", 31);
    Month June("June", 30);
    Month July("July", 31);
    Month August("August", 31);
    Month September("September", 30);
    Month Octomber("Octomber", 31);
    Month November("November", 30);
    Month December("December", 31);

    vector<Month> *myVect = new vector<Month>;
    myVect->push_back(January);
    myVect->push_back(February);
    myVect->push_back(March);
    myVect->push_back(April);
    myVect->push_back(May);
    myVect->push_back(June);
    myVect->push_back(July);
    myVect->push_back(August);
    myVect->push_back(September);
    myVect->push_back(Octomber);
    myVect->push_back(November);
    myVect->push_back(December);

    for(vector<Month>::const_iterator i = myVect->begin(); i != myVect->end(); i++)
    {
        /*
        Month myMonth = i;
        cout << myMonth.name << " " << myMonth.nr_days << endl;
        */
    }

    free(myVect);
    return 0;
}

I would want to be something like a foreach algorithm: foreach(Month in myVect) cout << ...

And another question: why it gives me a run-time error at the destructor if I uncomment my line?

Upvotes: 5

Views: 9036

Answers (6)

Ed Swangren
Ed Swangren

Reputation: 124770

Ok, there are a lot of problems here.

  1. You declare myVect as a pointer to a vector. This is unnecessary. One of the major benefits of using a vector is so that you don't have to worry about memory management as the vector does it for you. You stack allocate the vector, but internally it heap allocates the memory used to store the items it contains.

  2. You never initialize the pointer. You are invoking undefined behavior as that pointer is not valid. To initialize a pointer you use new. All you have is an invalid stack allocated pointer that does not point to a vector on the heap. EDIT: I just realized that the new was edited out, so you can disregard this one. Still, it shouldn't be a pointer at all.

  3. You are using free to deallocate a C++ class (that you never allocated to begin with...). Don't. This isn't C, you use new and delete to manage memory (when necessary!) in C++. free does not call destructors, it simply frees up a chunk of memory. delete on the other hand does as it knows how to deal with complex C++ types. Never mix new/delete with malloc/free.

  4. myVect->begin() returns a const_iterator, not a T (i.e., in this case, not a Month object). Dereferencing the iterator via the * operator will yield the current iteration object, so:

Month myMonth = *i  // <--- IMPORTANT!

As an aside, if you are going to be looping over the vector often you may want to typedef the iterator to reduce verbosity, i.e.,

typedef vector<Month>::const_iterator cmonth_iter;

Now you can write

for(cmonth_iter i = myVect.Begin(); i != myVect.end(); ++i )
{
    Month m = *i;
    // do stuff with m    
}

Upvotes: 7

6502
6502

Reputation: 114579

You can use the arrow operator with iterators...

for(vector<Month>::const_iterator i = myVect->begin(); i != myVect->end(); i++)
{
    cout << i->name << " " << i->nr_days << endl;
}

note also that it's more idiomatic with iterators using ++i instead of i++ (the reason is that i++ will need to create a copy of the iterator that will be thrown away).

Note also that your code is UB (undefined behavior) because you are using a pointer to a vector, but you are not allocating it. By the way the use of a pointer in this case is nonsense, the code would be correct and simpler with:

vector<Month> myVect;
myVect.push_back(January);
myVect.push_back(February);
...
for(vector<Month>::const_iterator i = myVect.begin(); i != myVect->end(); ++i)
  ...

My suggestion is also to avoid to try learning C++ just by experimenting with a compiler (something that I've the impression you're trying to do).

C++ is powerful but also complex and unfortunately quite illogical and asymmetrical in many parts (due to its evolution history). Add to this that when you make a mistake (e.g. not allocating the vector in your original code) you cannot expect the compiler to help you and even at runtime the program may do ANYTHING, including apparently work as you expected (the worst possible thing). This combo is deadly.

Complexity, asymmetry and lack of runtime checks all make C++ impossible to learn by experimentation... just get a good book and read it. It's much simpler this way.

Upvotes: 0

Adam Zalcman
Adam Zalcman

Reputation: 27233

You can access elements using iterator using the * operator:

for(vector<Month>::const_iterator i = myVect->begin(); i != myVect->end(); i++)
{
    Month myMonth = *i;
    cout << myMonth.name << " " << myMonth.nr_days << endl;
}

Also, you never allocate a vector in your code. You shouldn't use free() on a pointer you haven't received from malloc() earlier. It is undefined behavior to do otherwise and a run-time error is likely to occur at the point you call free().

Try this:

vector<Month> *myVect = new vector<Month>;
...
delete myVect;

Upvotes: 2

Mark Ransom
Mark Ransom

Reputation: 308520

You're declaring myVect as a pointer but never allocating it, that's going to give you lots of trouble. Just drop the * and you should be fine.

If you insist, you can use an index just like you would with an array:

for(int i = 0; i < myVect.size(); i++)
{        
    Month myMonth = myVect[i];
    cout << myMonth.name << " " << myMonth.nr_days << endl;
}

Although I'd rather use iterators as you have done - just one simple fix:

    Month myMonth = *i;

Upvotes: 0

Michael Krelin - hacker
Michael Krelin - hacker

Reputation: 143269

  1. You have a pointer myVect, but never assign a value to it before using (turn compiler warnings on). you should do something like myVect = new vector<Month>(). (or do not make it pointer and change -> into .). The rest of your "foreach" implementation looks fine. And you can use [] to access elements as well.

  2. You free constant strings, you did not allocate them, so you need not to free them either.

Upvotes: 0

Drew Dormann
Drew Dormann

Reputation: 63946

If you remove the unitialized pointer bug by changing:

vector<Month> *myVect;

to:

vector<Month> myVect;

Then this will work. (Once you define ostream << Month)

for(i = 0; i < myVect.size(); i++)
 cout << v[i] << " ";

Upvotes: 1

Related Questions