[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Understanding exceptions



On Saturday 21 February 2004 20:33, Florent Rougon wrote:
> Andrew Suffield <asuffield@debian.org> wrote:
> > *Yuck*
> >
> > This totally needs another layer of abstraction (it's also lacking
> > sufficient context to even guess at what that would look like;
> > probably some sort of message description data structure, and a
> > generic parser).
>
> The message description data structure is in the struct.unpack
> arguments. Example:
>
>   ">3s2s1s1s15s1s1s"
>
> >   means that integers are expected in big-endian order in the string
>
> 3s  means that the first object extracted is a string that is made of the
>     first three bytes encountered
> 2s  is similar (2 bytes)
> etc.
>
> (in this example, there are no integers, so the > has no effect; there
> are no integers either for the other message types: this part of the
> protocol was pure ASCII)
>
> This string allows you to unpack a stream of bytes into various objects,
> knowing that bytes n through m represent an unsigned 64 bits integer,
> bytes p through q a string, etc. As I said, this is "decoding of a
> network message".
>
> > This is the sort of code that I would make it a high priority to
> > replace. It's awful.
>
> Yeah, I didn't expect anything different from you. That is your way of
> arguing. We all know that.
>
> > You're comparing "bad code without exceptions" to "slightly less bad
> > code with exceptions". The alternative you describe is not a good way
> > to write this code either.
>
> You can obviously write it differently, but not much shorter nor
> clearer.
>
> > If I had written it, there would be precisely one unpack call, so
> > checking the return code would not be an issue.
>
> Because you don't understand what it does. You cannot factor every
> alternative in a generic function because for each message type,
> *different* fields of the high-level message structure (appli) are
> filled in. Your generic function will end up with a switch statement (or
> the if... elif... else equivalent) in order to assign the values to the
> right fields. Or you will make a dictionary with the keys being the
> message types and the values being the functions that will fill the
> corresponding fields in appli, but this only adds indirection and makes
> the code bigger in this case. I would do this way if there were more
> message types or if their handling was more complex, but this would be
> ludicrous in this case.
>
> I have the whole protocol subpart in 28 lines, straightforward to
> understand and *with* error handling (socket errors are handled by a try
> statement in an upper level). Why would I make it thrice as big? KISS.

Hi together,

while I agree more with you (Florent) than with Andrew in terms of this thread 
(exceptions...), I do agree with Andrew on this particular piece of code 
concerning readability/maintainability (so I don't argue about exceptions 
here). For me, KISS would be more like that:

msgdict = {231: None,
           258: {"bytestruct": ">3s2s1s1s15s1s1s",
                 "bytes": 24,
                 "fieldnames": ("field name 1", "field name 2",
                                "field name 3", "field name 4",
                                "field name 5", "field name 6",
                                "field name 7")},
           259: {"bytestruct": ">2s2s6s6s31s1s",
                 "bytes": 48,
                 "filenames": ("field name 2", "field name 8",
                               "field name 9", "field name 10",
                               "field name 4")},
           # ... and so on
           }

def setappli(appli):                    # member function
    msg = msgdict[self.header["type"]]
    if msg:
        fieldvalues = struct.unpack(msg["bytestruct"],
                                    sock.recv(msg["bytes"]))
        appli.update(dict(zip(msg["fieldnames"],
                              fieldvalues)))

But, as always, it's a matter of taste...

Regard,

	Tilo



Reply to: