Roy Smith wrote: > In article <52b782db$0$6599$c3e8da3$54964...@news.astraweb.com>, > Steven D'Aprano <steve+comp.lang.pyt...@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 -- https://mail.python.org/mailman/listinfo/python-list