views:

187

answers:

7
+5  A: 

I already answered it there. The rules for vtable instantiation are explained in your compiler documentation.

Here, it is waiting to see the definition (body) of Land::accept, which you declared to be a non-pure virtual, but never defined. Either define it, or make it pure virtual.

Useless
I think I get it now. Thank you very much.
Martin Kopta
+2  A: 

If you're not implementing a virtual function (i.e. if it is there to be overriden by descendants), you need to mark it as such via '=NULL' (it's then called pure virtual function).

class Land {
  public:
    virtual void accept(const Visitor *v)= 0;   // pure virtual function
};
Nicholaz
I think the common usage is = 0. Though in C++ NULL is defined as 0 you may confuse others with this notation and confusion is bad.
Martin York
Thanks. I didn't know that '=0' marks function to be pure virtual. Next lesson learned.
Martin Kopta
Martin is right, it should be "=0" ...
Nicholaz
+1  A: 

Implement Land::accept method or declare it as pure virtual.

However, I spotted a possible problem in main:

trip_plan.push_back(england);
trip_plan.push_back(russia);
trip_plan.push_back(england);

I don't know what type vector is, but you might have a problem providing derived class objects to be inserted in a vector of base class values (Type Slicing).

Cătălin Pitiș
Yep, there's even a SO question for that:http://stackoverflow.com/questions/274626/what-is-the-slicing-problem-in-c
Mattias Nilsson
Yes, I had problem with this but I used some pointers to get rid of it. But it confused me for a while.
Martin Kopta
That's the word I was unable to remember! Slicing!
Cătălin Pitiș
A: 

I'm seeing two more things:

1) note the missing "<" before size

void Trip::accept(Visitor *v) {
  for (unsigned i = 0; i < size(); i++) {
    l->at(i).accept(v);
  }
}

2) I think (assuming I understand your example correctly) vector should be vector< Land > (you're building a vector of abstract Lands which then is filled with pointers to concrete Land objects)

 vector<Land> Trip;

or

 typedef vector<Land> trip_t;  // type for a trip is a vector of Lands
 ... 

 trip_t Trip;

(It appears you're currently still editing the sample while I write this comment, so I'll have to go with a more general answer).

Nicholaz
Im sorry.. The mighty stackoverflow text processing cannot handle the < and >
Martin Kopta
A: 

I think that where you are using vector you should be using std::vector<Land*>:

class Trip {
  private:
    std::vector<Land*> *l;   // vector of pointers to Land
  public:
    explicit Trip(std::vector<Land*> *_l);
    void accept(Visitor *v);
};

and

void Trip::accept(Visitor *v) {
  for (unsigned i = 0; i< l->size(); i++) {
    l->at(i)->accept(v);  // . changed to ->
  }
}

and

int main() {
  England england;
  Russia russia;
  std::vector<Land*> trip_plan;
  trip_plan.push_back(&england);   // push_back pointers
  trip_plan.push_back(&russia);
  trip_plan.push_back(&england);
  Trip my_trip(&trip_plan);
  Visitor me;
  my_trip.accept(&me);
  return 0;
}

You need to use <Land*> so that you don't slice england and russia into instances of Land. Also, you might think about using an iterator in Trip::accept next time.

quamrana
A: 

Ok, here is a full working sample (not sure if your got clipped from yours). It's compiling here and I marked all the places where I made changes with "!!!" comments:

#include <cstdio>
#include <vector>

using namespace std;

class Visitor;

class Land {
  public:
    virtual void accept(const Visitor *v)= 0;  // !!! = 0
};

class England : public Land {
  public:
    void accept(const Visitor *v);
};

class Russia : public Land {
  public:
    void accept(const Visitor *v);
};

class Visitor {
  public:
    void visit(const England *e) const;
    void visit(const Russia *r) const;
};


class Trip {
  private:
    vector<Land *> *l;    // !!! <Land *>

  public:
    Trip(vector<Land *> *_l);   // !!! <Land *>
    void accept(Visitor *v);
};


/* Implementations */

void Visitor::visit(const England *e) const {
  printf("Hey, it's England!\n");
}

void Visitor::visit(const Russia *r) const {
  printf("Hey, it's Russia!\n");
}

void Russia::accept(const Visitor *v) {
  v->visit(this);
}

void England::accept(const Visitor *v) {
  v->visit(this);
}

Trip::Trip(vector<Land *> *_l) : l(_l)   // !!! <Land *>
{

}

void Trip::accept(Visitor *v) {

  for (unsigned i = 0; i < l->size(); i++) {     // !!! i < l->size()
    l->at(i)->accept(v);          // !!! at(i)->accept()
  }
}

int main() 
{
  England england;
  Russia russia;

  vector<Land *> trip_plan;   // !!! <Land *>

  trip_plan.push_back(&england);    // !!! &england
  trip_plan.push_back(&russia);  // !!! &russia
  trip_plan.push_back(&england);    // !!! &england

  Trip my_trip(&trip_plan);
  Visitor me;
  my_trip.accept(&me);

  return 0;
}
Nicholaz
+1  A: 

This is probably going far beyond what you are asking, but from a design standpoint, I think the land specific stuff should be inside the class of each land, i.e. it bugs me a bit to see the overloaded visit() function in Visitor.

The accept() member for Russia and England on the other hand is the same, and should be moved up into the Land.

Here is how I would structure this (have a look at the use of accept(), arrive() and name()):

#include <cstdio>
#include <vector>

using namespace std;

class Visitor;

class Land {

  public:
    virtual void accept(const Visitor *v); 

    virtual void arrive(void) const = 0;
    virtual const char *name(void) const = 0;

};

class England : public Land {
  public:
    void arrive(void) const;
    const char *name(void) const;
};

class Russia : public Land {
  public:
    void arrive(void) const;
    const char *name(void) const;
};

class Visitor {
  public:
    void visit(const Land *l) const;
};


class Trip {
  private:
    vector<Land *> *l;  

  public:
    Trip(vector<Land *> *_l);   
    void accept(Visitor *v);
};


/**** Implementations  *****/

// underlying Land

void Land::accept(const Visitor *v) {
  v->visit(this);
}


// England

const char *England::name(void) const {
  return "England"; 
}

void England::arrive(void) const {
  printf("Welcome to our lovely country, your passport please\n");
}


// Russia

const char *Russia::name(void) const {
  return "Russia"; 
}

void Russia::arrive(void) const {
  printf("Passport!!\n");
}


// Visitor

void Visitor::visit(const Land *l) const {
  l->arrive();
  printf("Hey, it'm in %s!\n", l->name());
}



// Trip

Trip::Trip(vector<Land *> *_l) 
  : l(_l)   // !!! <Land *>
{

}

void Trip::accept(Visitor *v) {

  for (unsigned i = 0; i < l->size(); i++) {    
    l->at(i)->accept(v);         
  }
}



/**** main *****/

int main() 
{
  England england;
  Russia russia;

  vector<Land *> trip_plan;  

  trip_plan.push_back(&england);    
  trip_plan.push_back(&russia);  
  trip_plan.push_back(&england);

  Trip my_trip(&trip_plan);
  Visitor me;
  my_trip.accept(&me);

  return 0;
}
Nicholaz