Closed Bug 576052 Opened 14 years ago Closed 11 years ago

Rename NS_ABORT_IF_FALSE() to NS_ASSERT()

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: cjones, Unassigned)

Details

Reason 1: the "IF_FALSE" part just adds cognitive overhead.  I get the condition inverted for ABORT_IF_FALSE() way more than I ever did with assert().

Reason 2: coming from an "assert()" background, I found it nonobvious that ABORT_IF_FALSE() was really what I wanted, and not NS_ASSERTION().

Bobby argues (from the perspective of only wanting fatal asserts from here on) that having ASSERT() and ASSERTION() coexist will lead to confusion and coders/reviewers possibly choosing ASSERTION() when they really wanted ASSERT().  That's a fair point, but my (glib) counter is that s/NS_ASSERTION/NS_ASSERT/ should be next on the list (once tinderbox assertions are fixed).  In the meantime, I think people are more likely to use NS_ASSERTION when they really want NS_ABORT_IF_FALSE, so I see the glass as half-full --- I think confusion around ASSERT and ASSERTION will lead to more fatal asserts.
I think the potential confusion is a worse problem than the current cognitive overhead. If "fixing assertions" is going to happen as soon as you seem to think it will (I'm not convinced!), then living with the cognitive overhead for just a bit longer won't be so bad, will it? :)
The cool kids use MOZ_ASSERT() nowadays.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.