If (x) nested checks are the best way to write this?

Question

There are places where I check valid pointers before performing an operation on them; these checks can be nested quite deep sometimes.

For example, I have

if (a) { if (a->b()) { if (a->b()->c()) { a->b()->c()->DoSomething(); } } } 

I really don't like the look of this. Is there a way to turn this into something more readable? Perfectly,

 if (a && a->b() && a->b()->c() ) { ... } 

It would be great, but obviously did not work.

EDIT - nvm example that I set DOES, as everyone noted. I checked it to see if this worked, but an error occurred in my test in my test. Spirit!

Answer 1

Why does the latter not work?

In C, && is a short circuit operator, so it is evaluated from left to right, and if any evaluation is false, the evaluation stops.

In fact, you can write:

 a && a->b() && a->b()->c() && a->b()->c()->DoSomething(); 

Answer 2

Quote from K & R ¹ :

Expressions Associated with && or || are evaluated from left to right and it is guaranteed that the evaluation will be stopped as soon as the truth or falsehood is known.

So the last example will work just fine, as WhirlWind noted .

---

¹ C programming language , second edition, p. 21.

Answer 3

Your second example works in C / C ++. This is a short circuit when the first value is FALSE.

Answer 4

You saw from other answers that using && will work and will briefly check when a null pointer is encountered.

The tough programmer in me likes to avoid repeating method calls for such tests, since he avoids the worry if they are idempotent or not. One option is to rewrite it like this:

 A* a; B* b; C* c; if ((a=a()) && (b=a->b()) && (c=b->c())) { c->doSomething(); } 

Verbose and a bit clumsy is acceptable, but at least you know that each method is called only once.

Answer 5

Why is "obviously not working"? Since the && operator evaluates only the correct member, if the left is valid, rewriting is completely safe.

Answer 6

Since you already received a direct answer to your question, I just mentioned that long chains of calls, like yours, have a smell of code, and you can consider a better design. In this case, such a construction may include the use of a null object template, so your call can simply boil down to:

 a->CDoSomething(); 

Answer 7

Chains work, but this is not necessarily the best answer in the general case, especially because it hides the point of failure. Instead, I would suggest smoothing out the tests by inverting the logic so that it fails.

 if (!pa) return Fail("No pa"); B* pb = pa->b(); if (!pb) return Fail("No pb"); C* pc = b->c(); if (!pc) return Fail("No pc"); pc->DoSomething(); 

The same, but flat and easy to read. In addition, since it immediately handles the case of a failure, it does not fall into the else , which you may never find to write.

In this example, I assumed that you do not want to simply fail, so I added Fail as an assistant, which registers the text and returns false. You can also just throw an exception. In fact, if various methods signaled their failure, throwing an appropriate exception instead of returning null, then all this would be unnecessary. If silent failure was desired, then a null object pattern would be appropriate.

Answer 8

if (a && a->b() && a->b()->c()) { a->b()->c()->DoSomething(); }

C ++ performs a lazy evaluation, so this will work. Firstly, a will be evaluated, and if it is 0, then the whole condition will be false, so there will be no evaluation of other parts.

This also works for the operator || . If you write if (a || b) , b will not be evaluated if a is true.

Answer 9

The second will work if you only want to call DoSomething() .