qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH 09/37] qapi/common.py: Add indent manager


From: John Snow
Subject: Re: [PATCH 09/37] qapi/common.py: Add indent manager
Date: Fri, 18 Sep 2020 12:08:19 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0

On 9/18/20 6:55 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:

We'll get to them in due time. For now, please admire the lipstick.

If I take off my glasses and step six feet back, I just might be able to
overlook it.


I consider writing a nice __repr__ good habit, I'd prefer not to delete it just because the rest of our code doesn't do so yet. (Give me time.)

I spend a lot of my time in the interactive python console: having nice __repr__ methods is a good habit, not an unsightly blemish.

+    def pop(self, amount: int = 4) -> int:
+        """Pop `amount` spaces off of the indent, default four."""
+        if self._level < amount:
+            raise ArithmeticError(
+                "Can't pop {:d} spaces from {:s}".format(amount, repr(self)))
I think assert(amount <= self._level) would do just fine.


Secretly, asserts can be disabled in Python just like they can in C code.

There are compilers that let you switch off array bounds checking.
Would you advocate manual bounds checking to protect people from their
own folly?

My habit: if it's something that should already be true given the
nature of how the code is laid out, use an assertion. If I am
preventing an erroneous state (Especially from callers in other
modules), explicitly raise an exception.

I check function preconditions ruthlessly with assert.  There's no sane
way to recover anyway.

Without a way to recover, the only benefit is crashing more prettily.
If the error is six levels deep in a some fancy-pants framework, then
prettier crashes might actually help someone finding the error slightly
faster.  But it ain't.

My final argument is local consistency: use of assertions to check
preconditions is pervasive in scripts/qapi/.


You're right that there's no safe recovery from an error such as this. The only actual difference is whether you see AssertionError or ArithmeticError.

One can be disabled (But you rightly shouldn't), the other can't. One has more semantic meaning and information to it.

I prefer what I've currently written.

--js




reply via email to

[Prev in Thread] Current Thread [Next in Thread]