John Snow <js...@redhat.com> writes: > On 9/17/20 4:07 AM, Markus Armbruster wrote: >> John Snow <js...@redhat.com> writes: >> >>> On 9/16/20 11:13 AM, Markus Armbruster wrote: >>>> John Snow <js...@redhat.com> writes: >>>> > > >>>> Let's replace "the indent" by "the indentation" globally. >>>> >>> >>> They're both cromulent as nouns and one is shorter. >>> >>> I'll switch in good faith; do you prefer "Indentation" as a noun? >> Use of "indent" as a noun was new to me, but what do I know; you're >> the >> native speaker. Use your judgement. Applies to the class name, too. >> > > I made the change; see gitlab commits or wait for v2 to see if it > reads better to you. > >>>>> + return self._level >>>>> + def __repr__(self) -> str: >>>>> + return "{}({:d})".format(type(self).__name__, self._level) >>>> Is __repr__ needed? >>>> >>> >>> Yes; it's used in the underflow exception , and it is nice when using >>> the python shell interactively. >>> >>> repr(Indent(4)) == "Indent(4)" >> Meh. There's another three dozen classes for you to put lipstick on >> :) >> > > 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. >>>>> + 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/. > (See the gitlab branch for the updated version of this patch, which > has changed the code slightly.) [...]