Is `if( hidden ) return;` an antipattern?

Sometimes, when you're doing something like displaying a bunch of elements, you can avoid running a function entirely if an object is hidden.

So you can either write the check from the loop:

for( Displayable * d : displayables )
    if( d->isShowing() )
        d->draw() ;

Or you can put an "early return" in d->draw() if d is hidden:

void Displayable::draw()
{
    if( !isShowing() )  return ;
    // continue with draw..
}

With this 2nd way, you don't have to check any condition when looping, so the loop "looks cleaner"

for( Displayable * d : displayables )
    d->draw() ;

But with the 1st way, you don't have to check isShowing() at the beginning of the function, so then that looks cleaner,

void Displayable::draw()
{
    // simply draw
}

Which is better? Is the latter an antipattern of sorts?

There's a semantic difference between the two of these. In the first, the draw method always draws the object whether the object is visible or not. I.e. it is the caller's responsibility to check the visibility of the object. In the second, the object checks it's own visibility and recognises that it does not need to do anything if the object is hidden.

The difference in the loop, then, is the difference between saying "Draw all of the non-hidden elements", and "Draw all of the elements, (where draw means draw the element if it is visible)"

One focuses on the fact that we're drawing visible elements, the other adds the concept of visbility to the method draw.

In my opinion, the second of these better fits the semantic of what draw is supposed to do, and for that reason it is clearly better to perform the visibility check inside draw rather than inside the loop.

StackExchange Code Review Q#36978, answer score: 10