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 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?

share|improve this question
1  
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 '12 at 22:33
add comment

migrated from stackoverflow.com Jan 26 '12 at 1:14

This question came from our site for professional and enthusiast programmers.

4 Answers

up vote 1 down vote accepted

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.

share|improve this answer
    
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 '12 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 '12 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 '12 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 '12 at 22:49
add comment

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.

share|improve this answer
add comment

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

share|improve this answer
add comment

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!

share|improve this answer
    
I see the issue. Any pointers or resources to the approach you mention ? –  curryage Jan 25 '12 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 '12 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 '12 at 23:21
add comment

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.