Object oriented design to model generic device-CodePudding

I was asked below object oriented system design question in an interview.

There are multiple devices like Echo show, Echo dot, Echo tab, Smart microwave, Fire tv stick etc.

Echo show - It has display screen and speaker. It works on electric power supply.

Echo dot - It has speaker. It works on electric supply.

Echo tab - It has speaker. It works on battery supply. Battery can be charged.

Smart microwave - It has screen display. It works on electric supply.

Fire tv stick - It has speaker. It works on electric supply.

So basically these are 3 categories like - speaker / screen display / both speaker and screen display There are two categories like - electric supply / battery supply. There can be queries on any of these devices like print status. Here are possible outputs for each of this device -

Echo show - "It is on charging" or "It is not charging" depending on whether it is connected to electric supply or not. This output should come on screen and speaker.

Echo dot - "It is on charging" or "It is not charging" depending on whether it is connected to electric supply or not. This output should come on speaker.

Echo tab - "Battery is charging" or "Battery is not charging and battery level is 70%" depending on whether battery is charging or not. This output should come on speaker.

Smart microwave - "It is on charging" or "It is not charging" depending on whether it is connected to electric supply or not. This output should come on screen.

Fire tv stick - "It is on charging" or "It is not charging" depending on whether it is connected to electric supply or not. This output should come on speaker.

Assume that there are inbuilt classes for speaking and printing on screen. If we pass string to these class objects they will do respective job.

Now write 4-5 classes to model this scenario.

Design should be extensible meaning if tomorrow any new device comes with a new combination then it can be implemented without creating any new class. So you should not be creating class for each device.

Here is my object oriented solution but interviewer is not happy with it particularly with vector<Output*> outputs. He was suggesting to use some design pattern instead of vector. Can you think of a better solution?

class Output {
   public:
    virtual void print(string);
};
class Display : public Output {
    DisplayScreen obj;
   public:
      void print(string str) { obj.print(str); }
};
class Speaker : public Output {
    Audio obj;
   public:
      void print(string str) { obj.print(str); }
};
class PowerSupply {
   public :
    virtual string get_status();
};
class BatteryPower : PowerSupply {
   bool isCharging;
   int chargeLevel;
   public :
     string get_status();
};
class ElectricPower : PowerSupply {
   bool isCharging;
   public :
     string get_status();
};
class Device {
    vector<Output*> outputs;//I used vector because Echo show has both display and speaker
    PowerSupply powersupply;
    Device(vector<Output> &outputs, PowerSupply powersupply) {
        this->outputs = outputs;
        this->powersupply = powersupply;
     }
};

CodePudding user response：

vector<Output> doesn't allow for inheritance because it stores Outputs directly instead pointers or references. If you store a Display or Speaker in the vector it will be sliced.

Since outputs are unique to each device, I would make that a vector of unique pointers:

std::vector<std::unique_ptr<Output>> outputs;

CodePudding user response：

If I were in this interview, I would talk at some length about how "model this" is an invalid directive. Every interface reflects the requirements of the consumer and the consumer is not described...

So we have to assume that there is something that needs to have a pointer to any of these things and then use any of the capabilities that it provides. Sure, Device is a good name for this.

A consumer is not going to consider all outputs to be the same so, this vector<Output> doesn't appear to be very useful. I would imagine that the consumer of the Device interface needs something like this:

class Device {
    public:
    virtual ~Device() = 0;

    // Get a pointer to the primary display (e.g., for playing video), or nullptr if there isn't one
    // It's only valid while this Device exists
    virtual Display *getPrimaryDisplay() {
        return nullptr;
    }

    // Get a pointer to the audio output, or nullptr if there isn't one
    // It's only valid while this Device exists
    virtual AudioOut *getAudioOutput() {
        return nullptr;
    }

    // Get a pointer to the charge status, or nullptr if there isn't one
    // It's only valid while this Device exists
    virtual ChargeStatus *getChargeStatus() {
        return nullptr;
    }

    // Send a notification or message to the user via the primary
    // channel (text display and/or text to speech)
    virtual void message(const std::string &msg);
}

Note that we use Interface Segregation to separate out the various capabilities that the consumer might want to use.

We also provide default implementations of these things so that when we add new ones, we don't invalidate existing Device implementations. If all the implementations are in the same source repo, though, we would want to make these pure virtual so that adding a new kind of service would force a decision about whether to support it or not for each device.

If we wanted to support both, then we'd provide a pure virtual interface and a base abstract implementation that returns nullptr for all services.

On the other hand, if we had to maintain binary backward compatibility to 3rd party Device implementations, then we would use a registration system instead of explicitly enumerating the possible services in this interface.

Then you need implementations of this interface for each device. For Echo show, for example:

class EchoShow : public Device {
   // implements Display
   std::unique_ptr<EchoShowDisplay> m_display;
   // implements AudioOut
   std::unique_ptr<EchoShowSpeaker> m_speaker;

   public:
   Display *getPrimaryDisplay() {
       return m_display.get();
   }

   AudioOutput *getAudioOutput() {
       return m_speaker.get();
   }

   // No ChargeStatus, since there's no battery -- inherit default nullptr

   void message(const std::string &msg) {
       m_display.showAlert(msg);
       // play bell and message at high priority to interrupt
       // any streaming audio, etc.
       m_speaker.queueSound("BELL", PRIORITY_NOTICE);
       m_speaker.queueSpeech(msg, PRIORITY_NOTICE);
   }