Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
1  
It's already over-engineered with that many layers of concrete classes. But why not ask him? –  itsbruce Nov 14 '13 at 9:42
    
Good question @itsbruce. He no longer works here, company policy is to not provide ex-member contact details. Nice to know your feeling is it is over engineered and I will assume my suggestion of going direct didn't make you think I was making an incorrect decision –  Dave Nov 14 '13 at 9:43
1  
The obfuscation you added does not help the question. If you can explain what each class actually does (or restore some of the original names), more specific answers can be given. It does look to be hinting towards a SOLID, testable design, despite the lack of interfaces. It seems the original developer did not want Class1 to know about Class3. –  CodeCaster Nov 14 '13 at 9:47
    
As an aside, if this is c# code, you don't need return = value;, rather just return value;. –  Rotem Nov 14 '13 at 9:53
    
Class3 MyClass3 = new Class did you mean to write Class3 MyClass3 = new Class3? –  gnat Nov 14 '13 at 10:05

3 Answers 3

up vote 19 down vote accepted

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.

share|improve this answer
    
Thanks, it makes sense about keeping class 1 and 3 separate (as you said, there is noway to know if necessary based upon the detail I provided). You are also right about there being a class 4, sadly also a class 5 and 6... Thank you. –  Dave Nov 14 '13 at 9:51
1  
@CodeCaster, not at this stage, there is a lot of code to go through and sadly his naming convention is IMO poor (I would call a property PeriodWidthHours, where he is calling it pwhvd (period width hour value double), so since I'm updating and refactoring as I go and I thought I'd ask if this needed to be addressed as well. –  Dave Nov 14 '13 at 9:54
3  
I make it a point not to modify program structure without a good reason on code produced by another programmer until I get to know it well enough. Most programmers have a reason for doing things the way they do, even if it is to cover up for bad design decisions made elsewhere. –  Neil Nov 14 '13 at 10:14
1  
Is this really Facade pattern or is it perhaps more aptly described as Bridge. I consider Facade to be only applied to 'an entire subsystem' represented by a single object. –  Andyz Smith Nov 14 '13 at 16:55
1  
the main idea in all these isn't abstraction its decoupling. An abstraction may be used to achieve that, but its not the goal. –  Keith Nicholas Nov 14 '13 at 21:59

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.

share|improve this answer

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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