2

So I have these header files:

Shape.h:

class Shape
{
public:
    Shape();
    virtual ~Shape();
    virtual double getArea();
    virtual void printDraw();
    bool isLegal(Shape shape);
};

Trig.h:

class Trig : public Shape
{
public:
    Trig(Point pointA, Point pointB, Point pointC);
    virtual ~Trig();
    virtual double getArea();//override
    virtual void printDraw();//override
    bool isLegal(Trig trig);//override
};

but when I try to implement Trig.cpp, I get errors. I tried :

Trig::Trig(Point pointA, Point pointB, Point pointC) {..}

I've looked through the internet, but I don't seem to do the inheritance properly. [My code MUST have header file that includes the declarations only!... Requirements of the assignment!]

I'm new in using both inheritance and virtual functions in C++ [I've used inheritance in Java]

And concerning the virtual functions.. is there any difference in their implementation than in normal functions ? [I understand the difference in the behavior].

Nadin Haddad
  • 337
  • 2
  • 3
  • 9
  • 4
    The first version `Trig::Trig(Point pointA...)` is basically correct for the implementation of the constructor. What does the exact code look like, and what errors do you get? – jogojapan Sep 15 '12 at 11:55
  • if i write : Trig::Trig(Point pointA, Point pointB, Point pointC) {..} i get the error: undefined reference to `Shape::Shape()' – Nadin Haddad Sep 15 '12 at 11:58
  • Why does the second constructor has 3 names? i.e. Trig::Shape::Trig ?? And yes, please post complete code and the warnings you are getting. – C.J. Sep 15 '12 at 11:58
  • 3
    Your `Trig` constructor will implicitly call a constructor of `Shape` (the default constructor by default), because it is derived from `Shape`. You must therefore implement at least one constructor for `Shape` as well. Have you done that? (The error message seems to indicate you haven't.) – jogojapan Sep 15 '12 at 12:01
  • well, when i try to implement the constructor for Shape, is says : undefined reference to `vtable for Shape' .... what's vtable ? – Nadin Haddad Sep 15 '12 at 12:15
  • 1
    It means, most likely, that you haven't implemented some of the other functions of `Shape` either. Make sure you implement them all (or don't declare them). – jogojapan Sep 15 '12 at 12:18
  • 1
    See http://stackoverflow.com/questions/1963926/when-is-a-v-table-created-in-c for general answers related to what a virtual table (vtable) is. – jogojapan Sep 15 '12 at 12:19

2 Answers2

4

In a comment you explain that

“i get the error: undefined reference to `Shape::Shape()'”

That means that you have forgotten to provide an implementation of that constructor.

Now, with the technical problem fixed, look at the design.

The first two methods suffer from two maladies: (1) although they should never mutate an object they are not declared const, so they cannot be called on a const object, and (2) the get prefix reduces readability, is more to type and has no general advantage. I’ve found a get prefix useful in some rare circumstances as a disambiguation device, but all usages I’ve seen by beginners have been inappropriate copying of a Java convention, which makes sense in Java but not in C++. So, instead of …

virtual double getArea();
virtual void printDraw();

do

virtual double area() const;
virtual void print() const;

Then, the method …

bool isLegal(Shape shape);

is wrong in so many ways… Ouch! But let’s start with the purely technical.

From a purely technical point of view it is needlessly inefficient to pass the argument by value, which incurs a copy operation for each call. Instead pass the object by reference. And make that a reference to const, in order to support const objects and rvalue objects as arguments:

bool isLegal(Shape const& shape);

Next, naming: isLegal is an ungood name because most any C++ object will be legal. It would have to be, say, pornographic and residing in a non-Western country, in order to become illegal. And I can’t for the life of me think of any way to make an object pornographic, or make it reside in some specific geographic region.

So,

bool isValid(Shape const& shape);

Next, low level design, there is no good reason to have that non-virtual method as an ordinary member function, because that requires that you call it on an object. But all the information it needs is in the ordinary argument. We can see this confusion in the derived class, where …

bool isLegal(Trig trig);//override

is not at all an override: it's technically an overload of the function name, which means, just a different function with the same name. There’s no virtuality here, no override. And it is not needed.

So, make that a static member function, which does not have to be called on an object:

static bool isValid(Shape const& shape);

Finally, higher level design, the whole machinery of C++ constructors and destructors is there in order to avoid such methods and checking.

The idea is that you …

  • Establish a valid object in every constructor.

  • Keep the object valid in every method.

Then the object simply can’t become invalid. This approach is called single phase construction, and the properties of the object that makes it “valid’ are known as the class’ class invariant. Which should be established by every constructor, and maintained by every method.

This means, final version, the isValid function IS REMOVED: it has no job to do, because that job is (properly) done by the constructor(s) and the methods.

Okay, there are some technical challenges with single phase construction, in particular how to do derived class specific initialization in a base class constructor. This is covered by the C++ FAQ. It’s often a good idea to read the FAQ.

Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
  • WOW... thank you for your help :) something about the design.. well.. the design was mainly provided by the course staff, they said it's a poor design, but we must stick to it because the purpose of the assignment is practicing specific things, and due to the lack of time, it's better to work with the given design. – Nadin Haddad Sep 15 '12 at 12:55
  • " It would have to be, say, pornographic and residing in a non-Western country, in order to become illegal. And I can’t for the life of me think of any way to make an object pornographic, or make it reside in some specific geographic region." Made my day :D – Adri C.S. Jan 15 '13 at 12:37
1

The first version is correct:

Trig::Trig(Point pointA, Point pointB, Point pointC) {..}

It will compile, but it will not link. The reason is that you have declared a parameterless constructor of Shape, but you failed to define it. Eiher add a definition

Shape::Shape() {..}

or if the default constructor is OK, remove the declaration from the Shape's header.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523