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.

I have a situation where I have to access several "sub-properties" of an object and find it quite appalling to write such code.

I was wondering how best to deal with this situation:

void Main()
{
    MyAClass aClass = new MyAClass();
    aClass.BClass.CClass.DoSomething();
}

In addition to it being an eyesore, there are no null checks, and if one were to do them, the code would look even more cluttered.

At the moment I create shortcuts, but they are a bit of a code smell too:

public class MyAClass
{
    public MyBClass BClass { get; set; }

    public void DoSomething()
    {
        if (BClass == null || BClass.CClass == null)
            throw new Exception();

        BClass.CClass.DoSomething();
    }
}

What's the best way to access multiple properties like this?

share|improve this question
    
So, you're asking how to access several sub-properties without accessing several sub-properties? And I really don't understand why do you find such code appalling, I think it's quite readable. –  svick Jan 18 '12 at 16:26
    
It's hard to comment on such an abstract example. If you need to be bothered about myAClass.BClass.CClass from outside (presumably from an instance of a class encompassing your A class...), then perhaps it's not just aesthetics but the overall object design is just bad and you should rethink it, if it's possible. –  Konrad Morawski Jan 18 '12 at 16:38
    
@svick In my case I have to do this frequently, so my entire class is littered with these chains. It get's real ugly when you have to use those calls in statements : for(int i = 0; i < myAClass.BClass.CClass.Count(); i++) { ... }. Ok perhaps not the best example, but it becomes a lot less readable once you have loads of these. –  Andre Jan 18 '12 at 20:30
    
@Morawski I am working on a service and our lead architect is exposing a set of persistent data/objects through an "Equipment" property. So you could have: if (Equipment.Ipc.IsOpen()) { Equipment.Ipc.InjectMessage(...); }. It just seems very repetitive. At the moment I create a reference to these frequently used items (such as Ipc) in my own class. It's ok, but I was wondering if there wasn't some design pattern that has an elegant way of dealing with such cases. –  Andre Jan 18 '12 at 20:40
add comment

4 Answers

The first approach violates the Law of Demeter and Tell, Don't Ask.

The point of OOP is not to be pretty, but to have a strong and simple way of achieving modularity. Modules are the means to apply divide and conquer to software development itself, where the conquest is achieved through implementation and the division is achieved through interfaces. Interfaces are all that really matters.

Nobody really cares how you implement something, as long as it is correct and reasonably fast. What's far more important is its handiness. Expecting user's of your code to go exploring your object graphs is not handy for them. Whatever components you wire together to achieve your goal is your business.

A user of your code just wants to DoSomething and your class allows them to do that. That is your contract. And the establishment (and upholding) of such a contract increases robustness, because you are free to change the way you accomplish that something without breaking any outside code.

share|improve this answer
    
Great answer. Thanks! –  Andre Jan 18 '12 at 20:27
add comment

This is really a design problem. The caller is required to have knowledge (i.e., "a dependency") on the internals of MyAClass - and in this case, not only on the internals of MyAClass, but also internals of the instance MyBClass that MyAClass holds, the instance of MyCClass that BClass holds, etc. There are several problems with this design - notably, it will be fragile and difficult to change (i.e., modifications to MyCClass could impact not only this caller, but also MyAClass, MyBClass, etc.), and you'll have an ownership/order-of-instantiation problem (i.e., as you mention, "no null checks") - this API requires that each of the classes has a non-null instance of each object already initialized, or worse yet, the property "getter" will do a null check and then instantiate one before returning the new object. This is really bad because what looks like a simple property "get" call, could result in some huge memory allocation or operation.

Long story short, if the caller really needs to have knowledge of these other classes, perhaps you can redesign the API, so that the caller allocates instances of each of the objects (either by an explicit "new" or via a factory), and passes them to MyAClass as constructor arguments, effectively making the caller the "owner".

Big-picture wise, I'd recommend reading a great book I've recently come across - "API Design for C++" by Martin Reddy - it's focused on C++, but the design issues are largely the same. You could also check out the "Effective C#: 50 Specific Ways to Improve Your C#" by Bill Wagner and "More Effective C#: 50 Specific Ways to Improve Your C#" by Bill Wagner for some useful tidbits on .NET API design.

share|improve this answer
    
Getter with loading logic are often used for Lazy Loading. Do you imply that Lazy Loading is not a good pattern by itself, or do you think there are better ways to implement it ? It might be helpful for the OP to see alternative if any, as his code might be lazy loading the sub-classes. –  Matthieu Jan 18 '12 at 17:00
2  
Absolutely that's what I'm saying. When I look at a "getter", I'm assuming (and I don't think I'm alone in this assumption) that I'm simply retrieving some data. When you start doing (potentially) expensive or error-prone things like object instantiation inside of a property "getter", I think you're asking for trouble. I think if you really want a lazy-loading scheme, then don't do it in a property getter, but inside of a class method, because I feel that calling a class method removes the assumption that what I'm asking for will be 'cheap'. That's my $0.02 anyway. –  Brandon Jan 18 '12 at 17:11
    
Cannot agree more with you =) –  Matthieu Jan 18 '12 at 17:13
    
BTW - thanks for the edits - putting the links to those books in - really appreciate it. I'll get the hang of this thing soon :) –  Brandon Jan 18 '12 at 17:17
add comment

You are right, it's a code smell and a potential problem (if any child in chain is null).

To avoid this you should follow the rule of code access level - I don't quite remember the statement, but the thing is:

Your method must do only one thing (single responsibility) and do not operate on the different levels of the objects.

In your example, since you have BClass instance as a member of MyAClass, DoSomething() method should only access properties/methods of that member and never properties of properties.

Potential resolution:

  1. Introduce method in MyBClass

    public class MyBClass
    {
        public MyCClass BClass { get; set; }
    
        public void DoSomething()
        {
            CClass.DoSomething();
        }
    }
    

    and call it in MyAClass.DOSomething()

  2. Consider refactoring the code and replacing MyBClass member with MyCClass.

share|improve this answer
add comment

Neglecting the fact this is a design problem, as has been mentioned in the other answers, there is a way to somewhat improve that code in C# when you can't add the method yourself. This is especially useful when you have to traverse even longer hierarchies.

void Main()
{
    MyAClass myAClass = new MyAClass();
    MyCClass inner = myAClass
        .IfNotNull( a => a.BClass )
        .IfNotNull( b => b.CClass );
    if ( inner != null )
    {
        inner.DoSomething();
    }
}

The code for the IfNotNull extension method is:

public static TInner IfNotNull<T, TInner>(
    this T source,
    Func<T, TInner> selector )
    where T : class
{
    return source != null ? selector( source ) : default( TInner );
}

Of course in a scenario where you would throw an exception anyhow and it would be an exception to encounter an object which is null along the path, you could just catch the NullReferenceException and act upon that in any way you want.

share|improve this answer
1  
Mad generic skillz. Pretty cool. Thanks for the answer. –  Andre Jan 18 '12 at 20:44
add comment

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.