Closed Bug 1286947 Opened 4 years ago Closed 4 years ago

Should not use a loop to get a single item from a set

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: bytesized, Assigned: bytesized)

Details

Attachments

(1 file)

I noticed this weird line when searching through the code base [1]:

      for (let anchorElement of anchors) {
        this._showPanel(notificationsToShow, anchorElement);
        break;
      }

I find this unconditional loop break to be very odd. Since the code does not loop, I would like to change this code such that it does not use a for loop.

[1] http://searchfox.org/mozilla-central/rev/e269c68bc594b4a858624d73f0d9eb002f3c0844/toolkit/modules/PopupNotifications.jsm#843
Patch replaces unnecessary loop with simply calling |next()| on the Set iterator.
Attachment #8771104 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8771104 [details] [diff] [review]
remove loop.patch

Review of attachment 8771104 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me
Attachment #8771104 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4680463ffdd7
Replace unconditional break in loop with code that does not use a loop. r=gijs
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4680463ffdd7
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.