Question

I was recently in a job interview and my interviewer gave me a modeling question that involved serialization of different shapes into a file.

The task was to implements shapes like circle or rectangles by first defining an abstract class named Shape and then implements the various shapes (circle, rectangle..) by inheriting from the base class (Shape).

The two abstract methods for each shape were: read_to_file (which was supposed to read the shape from a file) and write_to_file which supposed to write the shape into a file.

All was done by the implementation of that virtual function in the inherited shape (Example: For Circle I was writing the radius, for square I saved the side of the square....).

class Shape {
public:
    string Shape_type;

    virtual void write_into_file()=0;

    virtual void read_into_files()=0;

    Shape() {
    }
    virtual ~Shape() {
    }};
class Square: public Shape {
public:
    int size;
    Square(int size) {
        this->size = size;
    }
    void write_into_file() {
        //write this Square into a file
    }
    void read_into_files() {
        //read this Square into a file
    }
};

That was done in order to see if I know polymorphism.

But, then I was asked to implement two functions that take a vector of *shape and write/read it into a file.

The writing part was easy and goes something like that:

for (Shape sh : Shapes) {
    s.write_into_file();
}

as for the reading part I thought about reading the first word in the text (I implemented the serializable file like a text file that have this line: Shape_type: Circle, Radius: 12; Shape_type:Square...., so the first words said the shape type). and saving it to a string such as:

string shape_type;
shape_type="Circle";

Then I needed to create a new instance of that specific shape and I thought about something like a big switch

<pre><code>
switch(shape_type):
{
case Circle: return new circle;
case Square: return new square
......
}
</pre></code>

And then, the interviewer told me that there is a problem with this implementation which I thought was the fact that every new shape the we will add in the future we should also update int that big swicht. he try to direct me into a design pattern, I told him that maybe the factory design pattern will help but I couldn't find a way to get rid of that switch. even if I will move the switch from the function into a FactoryClass I will still have to use the switch in order to check the type of the shape (according to the string content i got from the text file).

I had a string that I read from the file, that say the current type of the shape. I wanted to do something like:

string shape_type;
shape_type="Circle";
Shape s = new shape_type; //which will be like: Shape s = new Circle

But I can't do it in c++.

Any idea on what I should have done?

Was it helpful?

Solution

In you factory you could map a std::string to a function<Shape*()>. At startup you register factory methods will the factory:

shapeFactory.add("circle", []{new Circle;});
shapeFactory.add("square", []{new Square;});
shapeFactory.add("triangle", []{new Triangle;});

In your deserialization code you read the name of the type and get its factory method from the factory:

std::string className = // read string from serialization stream
auto factory = shapeFactory.get(className);
Shape *shape = factory();

You've now got a pointer to the concrete shape instance which can be used to deserialize the object.

EDIT: Added more code as requested:

class ShapeFactory
{
private:
  std::map<std::string, std::function<Shape*()> > m_Functions;

public:

  void add(const std::string &name, std::function<Share*()> creator)
  {
    m_Functions.insert(name, creator)
  }

  std::function<Shape*()> get(const std::string &name) const
  {
    return m_Functions.at(name);
  }
};

NOTE: I've left out error checking.

OTHER TIPS

In C++, with

for (Shape sh : Shapes) {
    s.write_into_file();
}

you have object slicing. The object sh is a Shape and nothing else, it looses all inheritance information.

You either need to store references (not possible to store in a standard collection) or pointers, and use that when looping.

In C++ you would to read and write some kind of type tag into the file to remember the concrete type.

A virtual method like ShapeType get_type_tag() would do it, where the return type is an enumeration corresponding to one of the concrete classes.

Thinking about it, though, the question was probably just getting at wanting you to add read and write functions to the interface.

You could create a dictionary of factory functions keyed by a shape name or shape id (shape_type).

// prefer std::shared_ptr or std::unique_ptr of course
std::map<std::string, std::function<Shape *()>> Shape_Factory_Map;

// some kind of type registration is now needed
// to build the map of functions
RegisterShape(std::string, std::function<Shape *()>);
// or some kind of
BuildShapeFactoryMap();

// then instead of your switch you would simply
//call the appropriate function in the map
Shape * myShape = Shape_Factory_Map[shape_type]();

In this case though you still have to update the creation of the map with any new shapes you come up with later, so I can't say for sure that it buys you all that much.

All the answers so far still appear to have to use a switch or map somewhere to know which class to use to create the different types of shapes. If you need to add another type, you would have to modify the code and recompile.

Perhaps using the Chain of Responsibility Pattern is a better approach. This way you can dynamically add new creation techniques or add them at compile time without modifying any already existing code:

Your chain will keep a linked list of all the creation types and will traverse the list until it finds the instance that can make the specified type.

class Creator{
    Creator*next; // 1. "next" pointer in the base class
 public:
    Creator()
    {
    next = 0;
    }
    void setNext(Creator*n)
    {
        next = n;
    }
    void add(Creator*n)
    {
       if (next)
         next->add(n);
        else
          next = n;
    }
    // 2. The "chain" method in the Creator class always delegates to the next obj
    virtual Shape handle(string type)
    {
        next->handle(i);
    }
);

Each subclass of Creator will check if it can make the type and return it if it can, or delegate to the next in the chain.

I did create a Factory in C++ some time ago in which a class automatically registers itself at compile time when it extends a given template.

Available here: https://gist.github.com/sacko87/3359911.

I am not too sure how people react to links outside of SO but it is a couple of files worth. However once the work is done, using the example within that link, all that you need to do to have a new object included into the factory would be to extend the BaseImpl class and have a static string "Name" field (see main.cpp). The template then registers the string and type into the map automatically. Allowing you to call:

Base *base = BaseFactory::Create("Circle");

You can of course replace Base for Shape.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top