Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I'm creating a vector editing program in C++, and I need a Shape interface which other concrete classes will implement. There is a requirement that no implementation inheritance is allowed. The design doc says that if you need polymorphism, use interfaces. If you need code reuse, use composition.

The Shape interface is:

class Shape
{
  public:

   virtual void get_name()=0;
   virtual void set_name()=0;
   virtual void get_linewidth()=0;
   virtual void set_linewidth()=0;

   ...
   ...about 20 other getters/setters
   ...

   virtual void draw()=0;
   virtual int area()=0;
   virtual void rotate(int angle)=0;

}

The Circle class:

class Circle: public Shape
{
  string name;
  int line_width;
  int angle_of_rotation;
  int radius;
public:
  string get_name(){ return name; }
  string set_name(string name){ this->name=name; }
  ...
  ...about 20 other getters/setters
  ...

  int area()
  {
    return PI*pow(this->radius,2);
  }

}

I have no problem with this, except common properties have to be repeated for each type of shape! This is solved using inheritance, but I am not allowed to use that.

So, I create a ShapeProperties class

class ShapeProperties
{
  string name;
  int line_width;
  int angle_of_rotation;

public:

  string get_name(){ return name; }
  string set_name(string name){ this->name=name; }

  ...
  ...about 20 other getters/setters
  ...

}

and a properties() method for the interface:

virtual ShapeProperties* properties()=0;

A user would then do:

Shape *shape = new Circle();
shape->properties()->set_name("my shape");
shape->properties()->set_line_width(4);
int area = shape->area();

My question: Is this good design? Is this bad design? Are there any obvious problems? How could it be made better?

share|improve this question
1  
No inheritance is a stupid thing to do. Even terrible languages like Java allow single inheritance. –  DeadMG Feb 27 '12 at 8:44
    
That's the requirement. And it's not stupid, considering the fact that it is usually abused. Of course, there are many arguments on both sides. –  illmath Feb 27 '12 at 8:47
1  
Usually abused? Right. So because other people de-reference NULL on occasion, we should ban pointers in all cases for all people? That's a bad argument. The use, or not, of a given feature should be considered on a case by case basis, and the fact that it can be mis-used is no excuse for not using it when it is totally appropriate. –  DeadMG Feb 27 '12 at 8:54
2  
And that's a reason why the non-juniours shouldn't use it because....? As I previously stated, just because it can be mis-used is no reason to ban it, only to pay attention before you use it. As you can see in the problem in the question, it's more than possible to mis-use interface inheritance just as much. Oh, and it's also perfectly possible to mis-use dogma. –  DeadMG Feb 27 '12 at 9:05
3  
OK. You are faced with a problem which would clearly be best solved by implementation inheritance. Your only solution is a filthy hack. You only have two choices. One: Use filthy hack and create a bad program. Two: Look at the evidence in front of your face and consider the possibility that those respected persons made a mistake. If you want a good program, pick #2. If you'd rather lick boots, pick #1. Also, there are billions of bad languages, and having a language which does not have a feature means absolutely nothing whatsoever. en.wikipedia.org/wiki/Appeal_to_authority –  DeadMG Feb 27 '12 at 9:32

2 Answers 2

shape->properties()->set_name("my shape");

This is called train wreck, therefore it is better to avoid, because the method that needs to call properties() and then set_name() needs to know too much. Unless the shape is a data structure, and in your case doesn't seems to be.


If you must use method to get properties, then make them data structure (without any methods, and with all public member variables), and return const reference :

const ShapeProperties& properties() const

then the properties looks like :

struct ShapeProperties
{
  std::string name;
  int line_width;
  int angle_of_rotation;
  // ...
};
share|improve this answer
    
I mean no implementation inheritance. Edited the question. –  illmath Feb 27 '12 at 8:49
    
@illmath Ok, hope the answer now makes more sense –  BЈовић Feb 27 '12 at 8:57
    
I'll look into that. One problem that comes to mind: what if there needs to be some validation in the future e.g. the name has to be at least 8 characters long? For this reason, I kept it as a method set_name(string). –  illmath Feb 27 '12 at 9:06

It's a bad design because you have implemented a subclass for something that isn't a genuine subclass. A Circle is simply one example of a FunctionalShape, as opposed to a VertexShape. Examples of FunctionalShapes are circles, and also ellipses, and similar things. The function used to generate the shape is just data, and it's a bad idea to create different subclasses to represent different things which are just data.

What your code demonstrates is that the restriction to an interface is pointless, because your logic can be trivially reduced to not requiring a dynamic dispatch. Most of it could just be even a public member variable. Technically, you haven't used implementation inheritance, but in reality, you have, you've just coded it in a slightly different fashion.

The reality is that without using implementation inheritance, there is no way in C++ to make this code usable. There's a reason that that language feature exists and it's because not having it is dumb, and pointless, and makes your life very hard for no valid reason whatsoever.

Composition can re-use logic. It can't re-use class description, which is the part that you're endlessly (and pointlessly) repeating. There is no solution within your given constraints.

share|improve this answer
    
It is 'just data' for this simplified example. I will have to do validation on it e.g. ensure that the Shape's name has a certain length, ensure that line_width is not more than 10. So, it certainly is not a POD which you are implying! –  illmath Feb 27 '12 at 9:13
    
@illmath: The need to factor out a couple of setters is way different to factoring out the existence of the variables and factoring out all the getters and all the setters. –  DeadMG Feb 27 '12 at 9:16

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.