5
\$\begingroup\$

I found this question in Glassdoor and CareerCup, this is an OOP design question.

Many people claim to have solved it using different design patterns. I think It can be solved either with bridge pattern or decorator pattern. In the original question the base class is derived by 4 classes: 1. WoodChair 2. WoodTable 3. MetalChair 4. MetalTable

Can you please say:

Which design pattern will you use? or how (don't have to use a design pattern)

Original Question:

You are on a team that is creating a program to model stress on furniture. Your task is to model the behavior of furniture under abuse such as excessive weight or application of fire.

Someone has already created a prototype with a base class Furniture, and four derived classes WoodChair, SteelChair, WoodTable, and SteelTable.

We will need to start adding other furniture like couches, beds, bookcases, and desks, and also new materials such as plastic, cloth, rubber, etc. Try to improve the class design - you are free to modify it however you wish since it is only a prototype.

applyWeight(double x, double y, double weight, double seconds);

the furniture becomes unusable if enough weight is applied; the algorithm depends on shape of furniture, and the location where the weight is applied

applyFire(double x, double y, double fireStrength, double seconds); 

the furniture becomes unusable if it is made of wood, and fire is applied long enough; wood will change from brown to black if it is burnt (whether or not the furniture becomes unusable)

Color getColor(); // possible values: Gray, Brown, Black, etc
FurnitureState getState(); // possible values: OK, UNUSABLE<br>

Please comment on my implementation of the decorator Pattern. And OOP design.

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace JobInterviewTests
{
    [TestClass]
    public class DecoratorPatternTest
    {
        [TestMethod]
        public void DecoratorPatternUnitTest()
        {
            Furniture temp = new BaseChair();
            temp = new WoodDecorator(temp,10);
            temp.applyFire(0, 0, 20, 10);
            Assert.AreEqual(Color.Black, temp.getColor());
        }
    }

    public abstract class Furniture
    {
        protected Color _color;
        protected FurnitureState _state = FurnitureState.OK;
        protected double _breakThreshold;
        public abstract void applyWeight(double x, double y, double weight, double seconds);
        public abstract void applyFire(double x, double y, double fireStrength, double seconds);

        public Color getColor()
        {
            return _color;
        }
        public FurnitureState getState()
        {
            return _state;
        }
    }

    public enum FurnitureState
    {
        OK,
        UNUSABLE
    }

    public enum Color
    {
        Gray,
        Brown,
        Black
    }

    public class BaseChair : Furniture
    {
        public BaseChair()
        {
            _color = Color.Brown;
            _breakThreshold = 10.0;
        }

        public override void applyWeight(double x, double y, double weight, double seconds)
        {
            if (x * y * weight * seconds > _breakThreshold)
            {
                _state = FurnitureState.UNUSABLE;
            }
        }

        public override void applyFire(double x, double y, double fireStrength, double seconds)
        {

        }
    }

    public class BaseTable : Furniture
    {
        public BaseTable()
        {
            _color = Color.Gray;
            _breakThreshold = 15.0;
        }
        public override void applyWeight(double x, double y, double weight, double seconds)
        {
            if (x * y * weight * seconds > _breakThreshold)
            {
                _state = FurnitureState.UNUSABLE;
            }
        }

        public override void applyFire(double x, double y, double fireStrength, double seconds)
        {

        }
    }

    public abstract class BaseDecorator : Furniture
    {
        Furniture _furniture = null;
        protected BaseDecorator(Furniture furniture)
        {
            _furniture = furniture;
        }

        public override void applyFire(double x, double y, double fireStrength, double seconds)
        {
            _furniture.applyFire(x, y, fireStrength, seconds);
        }
        public override void applyWeight(double x, double y, double weight, double seconds)
        {
            _furniture.applyFire(x, y, weight, seconds);
        }
    }

    public class WoodDecorator : BaseDecorator
    {
        double _fireThreshold;
        public WoodDecorator(Furniture furniture, double fireThreshold)
            : base(furniture)
        {
            _fireThreshold = fireThreshold;
        }
        public override void applyFire(double x, double y, double fireStrength, double seconds)
        {
            if (x + y + fireStrength * seconds > _fireThreshold)
            {
                _color = Color.Black;
            }
            base.applyFire(x, y, fireStrength, seconds);
        }
    }
}
\$\endgroup\$
  • 3
    \$\begingroup\$ Is my implementation correct? does it break the conditions set by the interviewer? - if you are not sure about it then I'm afraid your question is off-topic. \$\endgroup\$ – t3chb0t Apr 28 '17 at 19:54
  • \$\begingroup\$ @t3chb0t I meant my implementation of the decorator pattern. I know my code is working. \$\endgroup\$ – InterviewsAreUs Apr 28 '17 at 20:07
  • \$\begingroup\$ @InterviewsAreUs Patterns aren't the golden hammer (which is an anti-pattern actually) for everything. Use your brain and analyze the requirements (regarding scalability in that particular case). \$\endgroup\$ – πάντα ῥεῖ Apr 28 '17 at 20:16
  • \$\begingroup\$ I think you are using the pattern correctly. This is a very nice interview question \$\endgroup\$ – Gilad Apr 28 '17 at 20:16
  • \$\begingroup\$ @πάνταῥεῖ this a glassdoor OOP question from a major company. I just thought of how I would have answered it, and I think this is a good way for scalability, if they add more materials or more types of Furniture or any other requirements. I just wanted to get other people thoughts on my code. \$\endgroup\$ – InterviewsAreUs Apr 28 '17 at 20:21
5
\$\begingroup\$

A decorator should never BE a kind of the decorated object. In your code the BaseDecorator inherits from Furniture and therefore has an own state (which is Color, FurnitureState, ...). This leads to the problem that the WoodDecorator sets its own _color to black while the decorated furniture stays brown!

Therefore a decorator should only be used in conjunction with an interface (see Wikipedia article).

In this case I think the decorator pattern shouldn't be used. You model an essential part (the material) of a furniture out into a decorator. For me a furniture consists of a specific material and therefore should have the material as a composition.

Don't call your classes BaseChair, just use Chair. When you have classes like BaseChair (or better Chair) they should always be abstract because you can't instantiate a Base-Thing, you always need a concrete thing (you do that right at the beginning with Furniture temp = new BaseChair()). To prevent this automatically make the constructor protected.

Empty method implementations (like applyFire) are always a code smell.

I would wrap the weight and fire stress in classes. This makes the interface more readable and type-safe. This would have prevented the mistake you made in BaseDecorator.applyWeight where you call _furniture.applyFire.

The interview question also says that in the future there will be more furniture types and more materials. So instead of using a decorator I would model the material explicitly. I would move the Fire-Stresstest to the material because it solely depends on the material. When a material can burn it implements the interface IBurnable.

So here is my implementation of the test, hope it helps:

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace FurnitureTest
{
    [TestClass]
    public class FurnitureTest
    {
        [TestMethod]
        public void WoodChairBurns()
        {
            Furniture f = new WoodChair();
            Assert.AreEqual(FurnitureState.Ok, f.FurnitureState);
            Assert.AreEqual(Color.Brown, f.Material.Color);

            f.ApplyFireStress(new FireStress { FireStrength = 10, Seconds = 20 });

            Assert.AreEqual(FurnitureState.Unusable, f.FurnitureState);
            Assert.AreEqual(Color.Black, f.Material.Color);
        }

        [TestMethod]
        public void SteelChairDoesNotBurn()
        {
            Furniture f = new SteelChair();
            Assert.AreEqual(FurnitureState.Ok, f.FurnitureState);
            Assert.AreEqual(Color.Gray, f.Material.Color);

            f.ApplyFireStress(new FireStress { FireStrength = 10, Seconds = 20 });

            Assert.AreEqual(FurnitureState.Ok, f.FurnitureState);
            Assert.AreEqual(Color.Gray, f.Material.Color);
        }

        [TestMethod]
        public void SteelChairBreaksUnderWeight()
        {
            Furniture f = new SteelChair();
            Assert.AreEqual(FurnitureState.Ok, f.FurnitureState);
            Assert.AreEqual(Color.Gray, f.Material.Color);

            f.ApplyWeightStress(new WeightStress { X = 5, Y = 5, Weight = 100 });
            Assert.AreEqual(FurnitureState.Unusable, f.FurnitureState);
            Assert.AreEqual(Color.Gray, f.Material.Color);
        }
    }

    class WeightStress
    {
        public double X { get; set; }
        public double Y { get; set; }
        public double Weight { get; set; }
    }

    class FireStress
    {
        public double FireStrength { get; set; }
        public double Seconds { get; set; }
    }

    enum FurnitureState
    {
        Ok,
        Unusable
    }

    enum Color
    {
        Gray,
        Brown,
        Black
    }

    interface IBurnable
    {
        FurnitureState ApplyFireStress(FireStress fireStress);
    }

    abstract class Material
    {
        public Color Color { get; protected set; }

        protected Material(Color color)
        {
            Color = color;
        }
    }

    class Wood : Material, IBurnable
    {
        public Wood() : base(Color.Brown)
        {}

        public FurnitureState ApplyFireStress(FireStress fireStress)
        {
            if(fireStress.FireStrength >= 10 && fireStress.Seconds >= 5)
            {
                Color = Color.Black;
                return FurnitureState.Unusable;
            }
            else
            {
                return FurnitureState.Ok;
            }
        }
    }

    class Steel : Material
    {
        public Steel() : base(Color.Gray)
        { }
    }

    abstract class Furniture
    {
        public Material Material { get; private set; }
        public FurnitureState FurnitureState { get; private set; }

        protected Furniture(Material material)
        {
            Material = material;
        }

        public void ApplyWeightStress(WeightStress stress)
        {
            if(BreaksUnderWeight(stress))
            {
                FurnitureState = FurnitureState.Unusable;
            }
        }

        protected abstract bool BreaksUnderWeight(WeightStress weightStress);

        public void ApplyFireStress(FireStress fireStress)
        {
            if (Material is IBurnable burnable)
            {
                FurnitureState = burnable.ApplyFireStress(fireStress);
            }
        }
    }

    abstract class Chair : Furniture
    {
        protected Chair(Material material) : base(material)
        { }

        protected override bool BreaksUnderWeight(WeightStress stress)
        {
            return (stress.X < 10 && stress.Y < 10 && stress.Weight > 10);
        }
    }

    class WoodChair : Chair
    {
        public WoodChair() : base(new Wood())
        { }
    }

    class SteelChair : Chair
    {
        public SteelChair() : base(new Steel())
        { }
    }

    abstract class Table : Furniture
    {
        protected Table(Material material) : base(material)
        { }

        protected override bool BreaksUnderWeight(WeightStress weightStress)
        {
            return (weightStress.X < 10 && weightStress.Y < 10 && weightStress.Weight > 20);
        }
    }

    class WoodTable : Table
    {
        public WoodTable() : base(new Wood())
        { }
    }
}
\$\endgroup\$
  • \$\begingroup\$ does your design change if I said you can't change the base class which store the original 4 functions? \$\endgroup\$ – InterviewsAreUs Apr 30 '17 at 21:02
  • \$\begingroup\$ Sure, because the preconditions change. But as it was said "you are free to modify it however you wish since it is only a prototype" I think this is the better solution. BTW this doesn't seem to be a C#-Question but a Java-Question because of Javaisms like Getters instead of properties. \$\endgroup\$ – urmeli May 1 '17 at 8:51
1
\$\begingroup\$

How it's been already mentioned in the comments: OO design is not about patterns, but how to model real-world problems in the world of objects and classes.

Here the question was to model "the behavior" under predefined set of conditions as "fire" and "weight".

And the secondary task behind the scenes was to improve the existing code. If you read the requrements carefully, you will see that these two methods clearly have some extra arguments:

  • weight tests do not depend on the duration
  • fire tests do not depend on the place coordinates

About the OO model:

The requirement with "the location where the weight is applied" was completely missed. If I were the interviewer, I'd definetely want to see it in the design.

About the decorator:

Extensive use of inheritance here breaks OCP (Open-Closed Principle). Which is a sign of a code smell for me. Generally, if you'd like to go with Decorator pattern - create an interface of the type you want to decorate and operate over it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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