Re: [lmi] [lmi-commits] master a889ef0 2/8: Assert some preconditions

[Vadim Zeitlin]

On Thu, 23 Aug 2018 10:57:42 +0000 Greg Chicares <address@hidden> wrote:

GC> Because I'm cross-compiling msw binaries and running them in 'wine',
GC> I have no usable debugger. When at() fails, I know only that a bounds
GC> error occurred somewhere. When LMI_ASSERT fails, I know where the
GC> failure occurred: i.e., it gives me the information I need.

 I think the real problem here is lack of usable debugger (which is not at
all insurmountable BTW, I have no problems using gdb with remote debugger
running under Wine). As a notorious purist, I find it regrettable that the
tool limitations should dictate the way the code is written. As a somewhat
pragmatic programmer, I also realize that sometimes this is unavoidable.
But in this particular case it really isn't: a debugger can be used with
Wine and I think using it would bring other, more important, benefits too.

GC> > However if you prefer to write it like this, then, IMO, it
GC> > would make sense to replace the call to at() with operator[].
GC> 
GC> This function no longer calls at(), but the asserted precondition
GC> remains, which is exactly what I want.
GC> 
GC> Even if at() were still called here, I'd see no need to replace it
GC> with operator[], because at() is cheap, while replacing it requires
GC> thought and opens up the possibility of introducing a new defect.
GC> I don't think of the assertion as an adjunct to at() that just makes
GC> it more informative--instead, I see it as enforcing a contract,
GC> which incidentally happened to guard a call to at().

 Yes, this is how I see it too. But the contract should be enforced either
by the caller or by the callee, I think it's confusing to do it in both
places. Again, I'm not going after the few milliseconds to save (although I
wouldn't want to pessimize this code more than necessary neither, it's
already quite slow), but personally I just find using an index check
followed by at() call wrong. After all, you won't write

        LMI_ASSERT(column < all_columns().size());
        LMI_ASSERT(column < all_columns().size());

on two lines in a row as this would be manifestly redundant and any person
reading the code containing it would feel compelled to remove one of the
lines. But

        LMI_ASSERT(column < all_columns().size());
        all_columns.at(column);

is basically exactly the same and, unsurprisingly, provokes the same kind
of cognitive imbalance in me.

 Regards,
VZ