1

I came across the following question on a website for interviews:

Here are 3 products: table, chair and bench. Product can be of material: wood, metal, and plastic. Design class structures for this.

My first thought was to have an abstract class Product with a protected Material class member as shown below. My actual question is after the code block below

#include <stdio.h>
#include <string>
using namespace std;

class Type {
    public:
    // Commented out virtual below. Was a typo !
    /*virtual*/void SetType(string s) {
        m_typeName = s;     
    }
    virtual void PrintType() { 
        printf("Type = %s\n",m_typeName.c_str()); 
    }
    virtual ~Type()=0;
    Type& operator=(const Type& rhs) {
        if ( &rhs != this ) {
            m_typeName = rhs.m_typeName;
        }
        return *this;
    }
    private:
    string m_typeName;
};
Type::~Type() {}

class Wood: public Type {
};

class Plastic: public Type {
};

class Product {
    public:
    Product():m_type(NULL) {
    }
    virtual void SetType(Type* t){
        m_type = t;
    }
    virtual void PrintInfo() {
        printf("I am %s product\n",m_productName.c_str());
        m_type->PrintType();
    }
    virtual ~Product()=0;
    protected:
    string m_productName;
    Type* m_type;
};
Product::~Product() {}

class Table: public Product {
    public:
    Table() { 
        m_productName = "table";
    }
};

class Chair: public Product {
    public:
    Chair() { 
        m_productName = "chair";
    }
};

int main()
{
    Plastic *p = new Plastic();
    p->SetType("plastic");

    Table *t = new Table();
    t->SetType(p);

    t->PrintInfo();

    delete t;
    delete p;

    return 0;
}

However, I have a feeling it could be designed better.

In general, if your compositions (Product in the example) contain compositions (Material in the example), what is a good design pattern?

1
  • 2
    Objects encapsulate data and behaviour. What behaviour are they meant to exhibit? I wonder whether the question is to do with multiple dispatch, i.e. choosing behaviour based on two different types. For example what are the packaging costs for a metal chair? What are the labelling regulations for a plastic table?
    – Peter Wood
    Jan 25, 2012 at 22:33

4 Answers 4

1

Well, I see a few problems(or at least thinks I would do differently) with your code/design:

It doesn't conform to the rule of three, since you have written your own operator= for Type, but use the compiler generated copy constructor. Sincy you don't do any manual resource management I would suggest ditching the manually written operator=, the compiler generated one should suffice nicely.

Why do you have avirtual methods to set a private variable? Since the variable can't be accessed from derived classes it doesn't really make sense to make the setter virtual.

I would suggest writing apropriate operator<< for streamout instead of having print methods which call printf.

You do have an implementation for your purely virtual destructors, right? since they still need to be called from the derived classes destructors

I would generally call APIs which require the user to call a specific method after initialising an object badly designed (of course there might be exceptions). So the type should really be a constructor parameter (and shouldn't an object of type Plastic know that it is a Plastic (so the default constructor should set the Type).

Using using namespace std; in what appears to be a header isn't really a good idea, since it pollutes the current namespace.

Using a raw pointer which is passed from outside of the object (your m_type) is kind of dangerous unless you can guarantee that the pointed to object isn't deleted before the lifetime of your object ends (which you can't in this case). One way to solve that is using managed pointers (std::shared_ptr, std::tr1::shared_ptr or boost::shared_ptr) and make a deep copy of the pointed to object if it is passed as a raw pointer (which would need you to add a virtual deep copy method to each baseobject and override it for every derived one). Another way is to have a design where you don't need the pointers, meaning you don't use any kind of hierarchy for Type.

Personally I would probably ditch most of the inheritance and have only a Type and a Product class, which have members expressing what they are (strings or enums (or whatever) depending on what seems best in that case), unless the question explicitely they should be seperate classes (or specifies functionality which mandates they should be). Unnecessarily convoluted designs are rarely helpful afterall.

I should also add that I do not think that this is a good interview question (at least unless the original question has more details then you passed on), since it lacks any details on how the classes should be used. This means that any design decisions are basically guesswork (which is why I would go the way of KISS and don't use much of a class hierarchy). So if that came up in an interview I would ask to clarify the requirements.

4
  • Regarding the operator=, I had added it in for the future possibility of having a member which requires deep-copy, but I should probably add it "on demand". <br> The virtual method to set a private variable is more of an editing oversight than deliberate design choice. <br> Yes, the constructor could set the type.
    – curryage
    Jan 25, 2012 at 21:36
  • 1
    @curryage: Well for an interview-question I would suggest avoiding such oversights. Having an explicit operator= in case you come to a point where you might need to write it yourself isn't a good idea, since it comes with the additional burden of having to change it each time you add a class member, so it is an unnecessary maintenance burdon and therefore reflects poorly on your design.
    – Grizzly
    Jan 25, 2012 at 22:26
  • Point taken. As you mentioned, in an interview, I would ask the requirements, but then maybe the point of asking an ill-defined question was to see what clarifying questions are posed in return and take off from there.
    – curryage
    Jan 25, 2012 at 22:35
  • @curryage: That is what I would hope for in that case, otherwise I'm not sure I would want to work there.
    – Grizzly
    Jan 25, 2012 at 22:49
2

The drawback of your approach is that you must explicitly make a new "Product" class for each product type. Since this is c++, you then need to know your products at compile time. Every time you add or remove a product, you must recompile.

A different design may be to have products be defined by data.

Look into a "Component Based" approach to facilitate "Data-driven" programming.

This way, the program wouldn't need to know the type of products it was dealing with. More importantly: The programmer wouldn't need to know about the products themselves. The data can be populated by someone at the company more familiar with the products, leaving the programmer to program!

3
  • I see the issue. Any pointers or resources to the approach you mention ?
    – curryage
    Jan 25, 2012 at 21:44
  • Just so that I understand, are you advocating an approach where you just do ProductBuilder *pb; pb->CreateProduct("Table","Plastic") ?
    – curryage
    Jan 25, 2012 at 21:50
  • 1
    Here's a good reference for components: cowboyprogramming.com/2007/01/05/evolve-your-heirachy It's geared towards gaming but the idea applies everywhere. And yes, your example is similar to what I'd propose. The problem with interview questions like this is that they're often very basic. Usually an interviewer is just trying to see you understand object oriented programming- throwing back a component-based design may give them the impression that you don't understand OOP, since it often encapsulates data in different ways than they were expecting.
    – Jon G.
    Jan 25, 2012 at 23:21
1

Without delving too deeply into architectures this can be seen as a good place for dependency injection, a variant on components. The basic solution to your problem would be to create an interface (aka pure virtual class) for the "material" and embed a pointer to one in your furniture class. When that furniture is constructed you pass in a material for it to use. Extend that with a Design interface for what kind of furniture it is and you're golden. Alternately your Design class holds the Material pointer which gets injected when it is created, which all then gets injected into the Furniture.

In this way your furniture can easily be extended to be made from concrete, styrofoam, or whatever else comes to mind and all without writing any new Furniture code but only new Materials. And should a sudden need for brick BBQ grills arise, you're ready with just a little work.

Here's a link to a really simple explanation of what dependency injection is, since it's a simple idea: Without delving into gaming architectures this can be seen as a good place for dependency injection, a variant on components. Here's a link to a really simple explanation of what dependency injection is, since it's a simple idea: http://jamesshore.com/Blog/Dependency-Injection-Demystified.html

1

From how you formulated that question, I do not see any indication that a concrete C++ program was requested. As such, as an interviewer, I'd be immediately suspicious if a candidate would give me any source code whatsoever in an attempt to answer the question.

I can only assume that the question itself contains insufficient requirements on purpose, because it's part of our everyday lives. You have to be able to a) identify the lack of requirements and b) discuss different options.

In summary, the whole interview question is probably intended to find out if the candidate can talk about class diagrams and mentally dissect different alternatives for solutions. What I'd expect from a good candidate would be a quick draft of a class diagram (f.ex. in UML, though not necessarily) and lots of questions.

Any source code being written at this point would score negatively for that matter.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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