0

I have a class that has a member with type vector<CCPoint>. I want to initialize this member in the constructor call, how can achieve it?

I made it like this:

.h

class A{
    public:
        A(vector<CCPoint> *p);
    private:
        vector<CCPoint> *p;
}

.cpp

A:A(){
    this->p = p;
}

call

Vector<CCPoint> *p = new Vector<CCPoint>;
A a = new A(p);
Cœur
  • 37,241
  • 25
  • 195
  • 267
Ferenc Dajka
  • 1,052
  • 3
  • 17
  • 45
  • So what (besides the obvious typos) is wrong with this? If there's something specific you don't like about this approach you need to tell us that. – Ernest Friedman-Hill Dec 08 '13 at 23:14
  • damn, I got null pointer exceptions, but now I see that I used the vector before I initialized it. that happens if you don't sleep for a few days. @polka to be honest I haven't used c++ much, so I used this rule: "add a * before every non primitive variable and it will work like Java" at least as far as I experienced it. I know it's rude… thanks for your answers anyways – Ferenc Dajka Dec 08 '13 at 23:41
  • 1
    _"add a * before every non primitive variable and it will work like Java"_ is a terrible rule. It won't work like Java, because C++ doesn't have a native garbage collector. – Chad Dec 16 '13 at 16:58
  • fair enough... but I'm familiar with Java – Ferenc Dajka Dec 16 '13 at 18:11

2 Answers2

4

This will leak memory, because nobody is deleting the vector you "new"-ed.

Also, why have a pointer to a vector at all? Are you worried about copying it into the constructor being expensive?

Change the member to be a vector:

class A{
    public:
        A(vector<CCPoint> p);
    private:
        vector<CCPoint> p;
}

Change the constructor to use the initialiser list:

A:A(vector<CCPoint> newP) : p(newP){
    // Empty
}

And call like this:

Vector<CCPoint> p;
A a(p);

Never, ever create an object with "new" unless you know exactly why you are doing so, and even then, reconsider.

Performance Note: Yes this may cause a vector copy to occur, depending on copy elision by the compiler. An alternative C++11 fancy pants solution would be to use move:

class A{
    public:
        A(vector<CCPoint> p);
    private:
        vector<CCPoint> p;
}

A:A(vector<CCPoint> newP) : p(std::move(newP)){
    // Empty
}

Vector<CCPoint> p;
A a(std::move(p)); // After this completes, 'p' will no longer be valid.
  • Why should anyone delete it if I need it later? ANd when I no longer use the class A the GC will delete it won't it? – Ferenc Dajka Dec 16 '13 at 16:40
1

There's an error in your cpp file, you're missing the second colon:

A::A() {

Also, you can directly initialize p using an initializer list like so:

A::A( vector<CCPoint>* _p ) :
p( _p )
{}

Not that there's any real advantage in using this for primitive types like pointers, but it's good convention. Does this answer your question? I'm not clear on what the problem is, exactly, based on your post.

wrren
  • 1,281
  • 9
  • 11