Re: Understanding exceptions
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.
> I wouldn't consider it adequetely useful. *Why* was there a decoding
> error? Which part of the message did not match the expected pattern?
> These messages are clearly not simple; figuring it out by hand would
> be unpleasent.
If struct.error is raised, it means the byte stream that was received
from the network contains an ill-formed message according to the
protocol (not matching ">3s2s1s1s15s1s1s" in the case of message type
258, for instance).
The error message will tell you why the byte stream does not match the
given pattern.
% python
Python 2.3.3 (#1, Dec 22 2003, 17:42:14)
[GCC 2.95.4 20011002 (Debian prerelease)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import struct
>>> struct.unpack(">3s", "foo")
('foo',)
>>> struct.unpack(">3s4s", "foo")
Traceback (most recent call last):
File "<stdin>", line 1, in ?
struct.error: unpack str size does not match format
This will be propagated to the upper layers and in the end, we will know
that a received packet was ill-formed. It would be trivial to let the
upper layer know the purported type of the ill-formed message. Just
change the:
except struct.error, val:
raise FooProgBodyDecodingError(val)
into:
except struct.error, val:
raise FooProgBodyDecodingError(self.header["type"], val)
and adapt the definition of the FooProgBodyDecodingError class
accordingly.
> And yes, that really is the whole point. You're throwing away useful
> information and then saying that it's simpler because you used an
> exception.
What information? With the aforementioned modification, the upper layer
will know:
- that a faulty message was received;
- the purported type of this message;
- why it was ill-formed.
This should be more than enough for the developper of the program that
spat the invalid message to fix his program. This is not enough? OK.
except struct.error, val:
raise FooProgBodyDecodingError(self.header["type"], appli_str, val)
Now, he will also know the exact contents of the buggy message he sent.
I cannot provide more. Note that I only added *two* words to the program
for this.
> It is not. It is simpler because it does less.
It does everything is conceivable for this task with the *two* words
added.
>> need to make the error handling sloppy in order to unobfuscate the
>> logic.
>
> Hah.
That is not an argument. The fact is that the logic is clear and similar
error handling can be factored out so that it does not obfuscate the
logic.
--
Florent
Reply to: