Object oriented design to model generic device
Asked Answered
P

3

5

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;
     }
};
Primer answered 22/12, 2021 at 14:25 Comment(4)
@463035818_is_not_a_number why do you paste link of object slicing? If I change vector<Output> to vector<Output*> then also interview is not happy about using vector.Primer
we cannot know what exactly made the interviewer unhappy, but std::vector<Output> is definitely wrong and std::vector<Output*> is ok-ish. I was considering to close the question as duplicate but there is already an answer that imho helps to explain better, so I just dropped the link.Clad
I changed "vector<Output> outputs" to "vector<Output*> outputs" to clarify confusion. Interviewer was suggesting to use some design pattern instead of vector.Primer
Sidenote: print(string) should be const. The problem, in my opinion, is that you didn't communicate with the interviewer: "Which pattern did you have in mind?" or "Which specific issue of vector should be targeted?". I'm personally not a fan of your justification for using vector. Why not make Device templated class with a non-type template parameter std::size_t kOutputCount, and a template specialization for Device<1ULL>? Also, this->outputs is a vector of Output* not of Output… Oh. And that constructor is privateLemmons
B
4

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;
Beery answered 22/12, 2021 at 14:46 Comment(2)
If I change vector<Output> to vector<Output*> then also interview is not happy about using vector.Primer
@Primer are you in the interview at this very moment?Clad
A
1

I do not think the interviewers request to use some design pattern was in any way constructive. Design patterns are tools to accomplish something; without a goal a design pattern is meaningless. Instead he could have said: "We are expecting to have to construct a lot of similar devices with minimal deviations. How would you accomplish that?" insted of "Now tag on a random design pattern and we are golden.".

Apart from the technical issue John Kugelman answered here. I think there are several design patterns you could have used instead of just passing in a vector of devices and power supplies. Some examples from the top of my head:

Factory pattern

Keywords for research: Factory pattern

class DotFactory
{
    DotFactory(/*maybe you need parameters*/);
    Device* getDevice(/*more parameters e.g. speaker count*/);
}

Builder pattern

Keywords for research: Builder pattern

Depending on the complexity and use case you could integrate the Device builder into the device. For simplicity I've done it this way.

class Device 
{
    // Your stuff
    vector<Output*> outputs;
    PowerSupply powersupply;
    Device(vector<Output*> &outputs, PowerSupply *powersupply);

    // Builder functions
    bool addPowerSupply(PowerSupply*);
    bool addOutput(Output*);

    // If outside your Device class also integrate:
    Device* getBuiltDevice();
};

Additional patterns and combinations (I was expecting to come up with more than one. Maybe someone can edit more in.)

Singleton list of devices (antipattern in most cases)

Depending on the usecase you could also have any thing (method, container, class etc.) holding your predefined 'Devices' if you only need one each per application.

Astra answered 27/12, 2021 at 10:11 Comment(0)
H
1

I suggest introducing the following 5 classes as shown below:

  • PowerStatusProvider -> interface/policy class for power status and such
  • BatteryStatus -> concrete implementation for all things battery powered
  • WiredStatus -> concrete implementation for all things powered by a wire
  • OutputHandler -> publish status to whatever endpoint using provided services (Screen, Speaker)
  • Device -> The actual abstraction to your gadget

I used structs instead of classes to shorten notation. The battery level is retrieved via a dependency injected functor. C++20 has std::format, maybe your compiler has that as well.


// Assuming Screen and Speaker are provided in a common interface 
struct OutputDevice{
    virtual void publish(std::string const & output) = 0;
    virtual ~OutputDevice()=default;
}

using OutputDevice_sp = std::shared_ptr<OutputDevice>;

struct Screen : OutputDevice{
    void publish(std::string const & output) override {
        // ...
        };
};

struct Speaker : OutputDevice{
    void publish(std::string const & output) override {
        // ...
        };
};

/// Here goes the actual implementation

struct PowerStatusProvider{
    virtual std::string to_string(bool Connected) const = 0;
    virtual ~PowerStatusProvider()=default;
};

struct BatteryStatus : PowerStatusProvider{
    BatteryStatus(std::function<double()> BatteryLevelProvider)
    : mBatteryLevelProvider(BatteryLevelProvider){}
    
    std::string to_string(bool Connected) const override
    {
        if (Connected)
            return "Battery is charging";
        else
            return std::format("Battery is not charging and battery level is {}%",mBatteryLevelProvider());
    }
private:
    std::function<double()> mBatteryLevelProvider;
};

struct WiredStatus : PowerStatusProvider{
    std::string to_string(bool Connected) const override
    {
        if (Connected)
            return "Device connected";
        else
            return "Device not connected";
    }
};

struct OutputHandler{
    OutputHandler(std::vector<OutputDevice_sp> Outputs)  
    : mOutputs(Outputs) {}
    
  void handle(std::string const & output) const {
      for (auto &&output : mOutputs) {
          output->publish(output);
      }
  } 

private:
    std::vector<OutputDevice_sp> mOutputs;
};

using PowerStatusProvider_sp = std::shared_ptr<PowerStatusProvider>;
using OutputHandler_sp = std::shared_ptr<OutputHandler>;


struct Device{
    struct Device(std::string Name, PowerStatusProvider_sp PSP, OutputHandler_sp OH)
    : mName(Name)
    , mPSP(PSP)
    , mOH(OH) 
    {}

    void update_status() const{
        mOH->handle(mPSP->to_string());
    }
  
private:
    std::string mName;
    std::shared_ptr<PowerStatusProvider> mPSP;
    std::shared_ptr<OutputHandler> mOH;
};
Hat answered 30/12, 2021 at 22:7 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.