Long delegation chains in C++

Question

This is definitely subjective, but I'd like to try to avoid it becoming argumentative. I think it could be an interesting question if people treat it appropriately.

In my several recent projects I used to implement architectures where long delegation chains are a common thing.

Dual delegation chains can be encountered very often:

bool Exists = Env->FileSystem->FileExists( "foo.txt" );

And triple delegation is not rare at all:

Env->Renderer->GetCanvas()->TextStr( ... );

Delegation chains of higher order exist but are really scarce.

In above mentioned examples no NULL run-time checks are performed since the objects used are always there and are vital to the functioning of the program and explicitly constructed when execution starts. Basically I used to split a delegation chain in these cases:

1) I reuse the object obtained through a delegation chain:

{ // make C invisible to the parent scope
   clCanvas* C = Env->Renderer->GetCanvas();
   C->TextStr( ... );
   C->TextStr( ... );
   C->TextStr( ... );
}

2) An intermediate object somewhere in the middle of the delegation chain should be checked for NULL before usage. Eg.

clCanvas* C = Env->Renderer->GetCanvas();

if ( C ) C->TextStr( ... );

I used to fight the case (2) by providing proxy objects so that a method can be invoked on non-NULL object leading to an empty result.

My questions are:

1. Is either of cases (1) or (2) a pattern or an antipattern?
2. Is there a better way to deal with long delegation chains in C++?

Here are some pros and cons I considered while making my choice:

Pros:

• it is very descriptive: it is clear out of 1 line of code where did the object came from
• long delegation chains look nice

Cons:

• interactive debugging is labored since it is hard to inspect more than one temporary object in the delegation chain

I would like to know other pros and cons of the long delegation chains. Please, present your reasoning and vote based on how well-argued opinion is and not how well you agree with it.

Answer 1

I wouldn't go so far to call either an anti-pattern. However, the first has the disadvantage that your variable C is visible even after it's logically relevant (too gratuitous scoping).

You can get around this by using this syntax:

if (clCanvas* C = Env->Renderer->GetCanvas()) {
  C->TextStr( ... );
  /* some more things with C */
}

This is allowed in C++ (while it's not in C) and allows you to keep proper scope (C is scoped as if it were inside the conditional's block) and check for NULL.

Asserting that something is not NULL is by all means better than getting killed by a SegFault. So I wouldn't recommend simply skipping these checks, unless you're a 100% sure that that pointer can never ever be NULL.

---

Additionally, you could encapsulate your checks in an extra free function, if you feel particularly dandy:

template <typename T>
T notNULL(T value) {
  assert(value);
  return value;
}

// e.g.
notNULL(notNULL(Env)->Renderer->GetCanvas())->TextStr();

Answer 2

In my experience, chains like that often contains getters that are less than trivial, leading to inefficiencies. I think that (1) is a reasonable approach. Using proxy objects seems like an overkill. I would rather see a crash on a NULL pointer rather than using a proxy objects.

Answer 3

Such long chain of delegation should not happens if you follow the Law of Demeter. I've often argued with some of its proponents that they where holding themselves to it too conscientiously, but if you come to the point to wonder how best to handle long delegation chains, you should probably be a little more compliant with its recommendations.

Answer 4

Interesting question, I think this is open to interpretation, but:

My Two Cents

Design patterns are just reusable solutions to common problems which are generic enough to be widely applied in object oriented (usually) programming. Many common patterns will start you out with interfaces, inheritance chains, and/or containment relationships that will result in you using chaining to call things to some extent. The patterns are not trying to solve a programming issue like this though - chaining is just a side effect of them solving the functional problems at hand. So, I wouldn't really consider it a pattern.

Equally, anti-patterns are approaches that (in my mind) counter-act the purpose of design patterns. For example, design patterns are all about structure and the adaptability of your code. People consider a singleton an anti-pattern because it (often, not always) results in spider-web like code due to the fact that it inherently creates a global, and when you have many, your design deteriorates fast.

So, again, your chaining problem doesn't necessarily indicate good or bad design - it's not related to the functional objectives of patterns or the drawbacks of anti-patterns. Some designs just have a lot of nested objects even when designed well.

---

What to do about it:

Long delegation chains can definitely be a pain in the butt after a while, and as long as your design dictates that the pointers in those chains won't be reassigned, I think saving a temporary pointer to the point in the chain you're interested in is completely fine (function scope or less preferably).

Personally though, I'm against saving a permanent pointer to a part of the chain as a class member as I've seen that end up in people having 30 pointers to sub objects permanently stored, and you lose all conception of how the objects are laid out in the pattern or architecture you're working with.

One other thought - I'm not sure if I like this or not, but I've seen some people create a private (for your sanity) function that navigates the chain so you can recall that and not deal with issues about whether or not your pointer changes under the covers, or whether or not you have nulls. It can be nice to wrap all that logic up once, put a nice comment at the top of the function stating which part of the chain it gets the pointer from, and then just use the function result directly in your code instead of using your delegation chain each time.

Performance

My last note would be that this wrap-in-function approach as well as your delegation chain approach both suffer from performance drawbacks. Saving a temporary pointer lets you avoid the extra two dereferences potentially many times if you're using these objects in a loop. Equally, storing the pointer from the function call will avoid the over head of an extra function call every loop cycle.

Answer 5

For bool Exists = Env->FileSystem->FileExists( "foo.txt" ); I'd rather go for an even more detailed breakdown of your chain, so in my ideal world, there are the following lines of code:

Environment* env = GetEnv();
FileSystem* fs = env->FileSystem;
bool exists = fs->FileExists( "foo.txt" );

and why? Some reasons:

1. readability: my attention gets lost till I have to read to the end of the line in case of bool Exists = Env->FileSystem->FileExists( "foo.txt" ); It's just too long for me.
2. validity: regardles that you mentioned the objects are, if your company tomorrow hires a new programmer and he starts writing code, the day after tomorrow the objects might not be there. These long lines are pretty unfriendly, new people might get scared of them and will do something interesting such as optimising them... which will take more experienced programmer extra time to fix.
3. debugging: if by any chance (and after you have hired the new programmer) the application throws a segmentation fault in the long list of chain it is pretty difficult to find out which object was the guilty one. The more detailed the breakdown the more easier to find the location of the bug.
4. speed: if you need to do lots of calls for getting the same chain elements, it might be faster to "pull out" a local variable from the chain instead of calling a "proper" getter function for it. I don't know if your code is production or not, but it seems to miss the "proper" getter function, instead it seems to use only the attribute.

Answer 6

Long delegation chains are a bit of a design smell to me.

What a delegation chain tells me is that one piece of code has deep access to an unrelated piece of code, which makes me think of high coupling, which goes against the SOLID design principles.

The main problem I have with this is maintainability. If you're reaching two levels deep, that is two independent pieces of code that could evolve on their own and break under you. This quickly compounds when you have functions inside the chain, because they can contain chains of their own - for example, Renderer->GetCanvas() could be choosing the canvas based on information from another hierarchy of objects and it is difficult to enforce a code path that does not end up reaching deep into objects over the life time of the code base.

The better way would be to create an architecture that obeyed the SOLID principles and used techniques like Dependency Injection and Inversion Of Control to guarantee your objects always have access to what they need to perform their duties. Such an approach also lends itself well to automated and unit testing.

Just my 2 cents.

Answer 7

If it is possible I would use references instead of pointers. So delegates are guaranteed to return valid objects or throw exception.

clCanvas & C = Env.Renderer().GetCanvas();

For objects which can not exist i will provide additional methods such as has, is, etc.

if ( Env.HasRenderer() ) clCanvas* C = Env.Renderer().GetCanvas();

Answer 8

If you can guarantee that all the objects exist, I don't really see a problem in what you're doing. As others have mentioned, even if you think that NULL will never happen, it may just happen anyway.

This being said, I see that you use bare pointers everywhere. What I would suggest is that you start using smart pointers instead. When you use the -> operator, a smart pointer will usually throw if the pointer is NULL. So you avoid a SegFault. Not only that, if you use smart pointers, you can keep copies and the objects don't just disappear under your feet. You have to explicitly reset each smart pointer before the pointer goes to NULL.

This being said, it wouldn't prevent the -> operator from throwing once in a while.

Otherwise I would rather use the approach proposed by AProgrammer. If object A needs a pointer to object C pointed by object B, then the work that object A is doing is probably something that object B should actually be doing. So A can guarantee that it has a pointer to B at all time (because it holds a shared pointer to B and thus it cannot go NULL) and thus it can always call a function on B to do action Z on object C. In function Z, B knows whether it always has a pointer to C or not. That's part of its B's implementation.

Note that with C++11 you have std::smart_ptr<>, so use it!