cascading python executions only if return code is 0

[Steven D'Aprano]

Roy Smith wrote:

> In article <52b782db$0$6599$c3e8da3$5496439d at news.astraweb.com>,
>  Steven D'Aprano <steve+comp.lang.python at pearwood.info> wrote:
> 
>> Code that relies on side-effects is usually a sign of poor
>> design.
> 
> I don't understand what you're trying to say there.

I'm trying to say that code that relies on side-effects is usually a sign of
poor design. Is that more clear now? :-)

I'm not a functional programming zealot, but imperative programming that
relies on side-effects is often harder to reason about than functional
style, due to lack of locality in its effects. With functions that
communicate only through their input arguments and output result, you don't
have to look far to see the effects the function call has: it is *only* in
the return result. But if it has side-effects, you potentially have to
inspect the entire program and environment to see what it has done.

Now of course sometimes the whole point of the function is to have a
side-effect (print something, delete or save a file, start up the car's
engine, ...) and even functional languages usually have some facility for
side-effects. And we can often limit the harm of relying on side-effects
but narrowly constraining what those side-effects are.

E.g. list.append has a very narrow and well-defined side-effect, which makes
it relatively easy to reason about it. But still not as easy as perhaps we
would like:

alist = blist = [1, 2, 4, 8]
# later on
alist.append(16)  # operates by side-effect
# and still later on
assert blist == [1, 2, 4, 8]  # FAILS!!!


A side-effect free language might make list.append return a new list with
the extra value appended, and then the above would not occur. But I
digress.

The point is, I'm not saying that imperative code that operates via
side-effects is always harmful. There are degrees of badness.


> A bit later in your 
> post, you wrote:
> 
> try:
>     a()
>     b()
>     c()
> except SomeError:
>     handle_error()
> 
> Clearly, since the return values of a(), b(), and c() aren't saved, the
> only reason they're getting called is for their side effects.

That's not my design :-)


> And I don't see anything wrong with that.

And quite frankly, neither do I. But I don't know what a, b and c actually
do.


> BTW, there's a pattern we use a bunch in the Songza server code, which
> is sort of this, but in reverse.  We'll have a bunch of possible ways to
> do something (strategies, to use the pattern vernacular), and want to
> try them all in order until we find one which works.

Sounds reasonable. You're not operating by side-effect, since you actually
do want the result generated by the strategy. Presumably the strategy
signature is to return None on failure, or instance on success. (I see
below that's exactly what you do.)


> So, for example: 
> 
>         classes = [ClientDebugPicker,
>                    StatefulSongPicker,
>                    SWS_SequentialSongPicker,
>                    StandardSongPicker]
>         for cls in classes:
>             picker = cls.create(radio_session, station, artist)
>             if picker:
>                 return picker

This seems perfectly reasonable up. The strategy either returns a picker, in
which case you are done, or it returns None and you continue. No
side-effects are involved.


>         else:
>             assert 0, "can't create picker (classes = %s)" % classes

¡Ay, caramba! I was with you until the very last line. The above code is
possibly buggy and inappropriately designed. (I may be completely
misinterpreting this, in which case feel free to ignore the following
rant.)

First, the bug: there are circumstances where no exception is raised even if
all the strategies fail. (If the StandardSongPicker is guaranteed to
succeed, then obviously my analysis is wrong.) In this case, then any code
following the for-else will execute. Since this is in a function, and there
doesn't seem to be any following code, that means that your function will
return None instead of a valid picker. Does the rest of your code check for
None before using the picker? If not, you have a bug waiting to bite.

Second, the poor design. When it works as expected, failure is indicated by
AssertionError. Why AssertionError? Why not ImportError, or
UnicodeEncodeError, or ZeroDivisionError, or any other of the dozens of
inappropriate errors that the code could (but shouldn't) raise? I guess
nobody here would write code that looks like this by design:

try:
    picker = select_picker(radio_session, station, artist)
except EOFError:
    handle_no_picker_case()


just because "raise EOFError" required less typing than raising some more
appropriate exception. So why would we write:

try:
    picker = select_picker(radio_session, station, artist)
except AssertionError:
    handle_no_picker_case()


just because "assert" required less typing than raising some more
appropriate exception? assert is not a short-cut for "raise
SomeGenericException". AssertionError has a specific meaning, and it is
sloppy to misuse it. An assertion error means that an assertion has failed,
and assertions only fail when your code contains a logic error or a program
invariant is violated. Both should never happen in production code, and if
they do they ought to be unrecoverable errors.

Anyway, I may be completely misinterpreting what I'm reading. Perhaps the
assertion is checking a function invariant ("one of the strategies will
always succeed") in which case you're doing it exactly right and I should
shut up now :-)


-- 
Steven