BaseRect::IsZero needs a rename to make its meaning clearer

RESOLVED FIXED in Firefox 59

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dholbert, Assigned: milan)

Tracking

(Blocks 2 bugs)

Trunk
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment)

It's non-obvious what BaseRect::IsZero means, when reading code that uses it.

If I see...
 if (r.IsZero()) {
   doStuff();
 }

...it's not obvious which members are being checked for zeroness (and whether all have to be zero or if only one is enough).

I think we should rename it to IsZeroArea(). That makes its meaning (width==0||height==0) clearer.

(We *might* want to rename the subtly-different IsEmpty as well to e.g. IsEmptyArea, but I'd say we should probably leave that one alone -- "empty" is clearer on its own. It's obviously size-related and it's obvious that either dimension is sufficent to trigger "emptiness".)
Bas also suggested we do an audit of the places that currently call IsZero and see if those should be replaced by IsEmpty.
Flags: needinfo?(milan)
IMO we should still do this rename first, since it's trivial & a clear readability win with no behavior impact, and then we can audit usages (& make potentially-behavior-changing edits) afterwards/elsewhere.

(feel free to hold on this until after bug 1424382 lands, to avoid causing bitrot in the massive-patch there. Though I guess it'd be a pretty trivial search-and-replace so either order is probably fine.)
Blocks: 1424371, 1424382
Flags: needinfo?(milan)
Assignee: nobody → milan
Comment on attachment 8941653 [details]
Bug 1429602: Rename BaseRect::IsZero to BaseRect::IsZeroArea.  Also slip in some corrections to using BaseRect methods instead of direct member access. .schouten

https://reviewboard.mozilla.org/r/211900/#review217692
Attachment #8941653 - Flags: review?(bas) → review+
Pushed by msreckovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0d48950798f4
Rename BaseRect::IsZero to BaseRect::IsZeroArea.  Also slip in some corrections to using BaseRect methods instead of direct member access. r=bas.schouten
https://hg.mozilla.org/mozilla-central/rev/0d48950798f4
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.