Is this an example of 'over engineered' or a good practice?

Question

The code I've inherited has a design that I'm not familiar with (I'm still new to the world of programming).

It is a .net project and there are 3 classes in question here.

public Class1
{
    public void DoSomething()
    {
        Class2 class2 = new Class2();
        string myValue = class2.GetSomeValuePlease();

       //why would I not do class2.MyClass3.GetActualValue();
    }
}

public Class2
{
    public Class3 MyClass3 = new Class3();
    public string GetSomeValuePlease()
    {
        return this.MyClass3.TheActualValue();
    }
}

public Class3
{
    public string TheActualValue()
    {
        return "This is the value";
    }
}

As you can see, all 3 classes are public. I don't understand why, using the example above, I would use Class2 at all? I could understand if within the GetSomeValuePlease() method there was some logic which affected Class2, but there isn't.

Normally, I'd just go ahead and remove the Class2 method (GetSomeValuePlease) and call Class3 method (GetActualValue) direct from Class1, but, the developer I took over (who is not contactable) is wiser and more experienced than I. I have a feeling this is just over engineering and this is just extra code / extra maintenance.

Does any one have experience in designing this way that could explain the thought process or implications of having this "middle class" vs going direct?

Answer 1

Looks like the purpose of Class2 is to abstract away the implementation of Class3.

This allows Class3 to be changed in the future. Maybe the value will be grabbed from a database, maybe it will be read from a local file, maybe it will be read from the web. Maybe it won't even be read from Class3 at all, maybe it will be read from Class4.

The abstraction makes it possible to change the implementation of getting a correct value without Class1 having to change any code.

This is a valid design pattern (see Facade Pattern Bridge Pattern). To say whether or not it is necessary, you'd need to provide the actual classes.

Answer 2

It depends. The good news is that if you understand the refactoring, you can reverse your decision later.

The advantage of the code as written is that you have more flexibility to change the internal structure of Class2 or change Class3 without affecting Class1.

The disadvantage is that every time you want to add a feature to Class3 that is used by Class1, you have to add a delegate method to Class2.

I suggest that you trust your instincts. You can use Remove the Middle Man to have the Class1 call Class3 directly.

If later you decide you should go back to the original design, you can use Hide Delegate.

I recommend the book Refactoring: Improving the Design of Existing Code by Martin Fowler.

Answer 3

This is the application of the "Law of Demeter" (which is not so much a law, but a good guide)

http://en.wikipedia.org/wiki/Law_of_Demeter

The basic idea is not to reach through objects to get access to other objects as that increases your coupling with an eco system of objects, meaning, changes are likely to ripple through entire systems.

However, I have seen people go too far with this and end up building facades to other objects into objects that have their own responsibility.

Most often if you find you want to apply "LoD" and find you are doing this kind of delegating / facade / Bridge type stuff, start having a good think about whether your design is right, because quite often a slight adjustment to the design / responsibilities will untangle the need to reach through one object to get to other objects.