HomePhabricator

Simplify loadCallback code flow

Description

Simplify loadCallback code flow

  • loadEL: There were a few cases where this was conditionally assigned based on other prior decisions. This felt a bit fragile. To avoid breaking this in the future and to make to easier to understand each individual statement, refactor it so that it can be assigned in an idiomatic 'ensure set' way. Meaning, "set x if x is not already set". With other circumstances ensuring we only reach it if need be (with an outer conditional, and/or early return). Previously this was inferred by carefully repeating certain conditions where needed.
  • reinit (test code): Update to account for oversampling. Wasn't actually broken in current tests, but by using !loadEL, naturaly works.
  • Remove length checks before forEach(). This isn't something we typically do in JavaScript, but also not needed here given there is no shared body of code for the condition beyond the loop. The outcome of the pushes is asserted elsewhere already. This saves a few extra variables and simplifies the code flow.
  • Remove Object.keys() check before for-loop. The main code below it already accounts for it being empty by producing an empty array, which make the check redundant. Also, in JS computing Object.keys() relatively expensive (more than for-loop itself). Super tiny either way though, but given it effectively means we iterate the whole object to decide whether to start iterating it (again), removed and simplifies the code :)
  • Update getGeo/getUA oversampling checks to not create two arrays where one isn't used. Instead, return the one created earlier, or, allocate the main one later. I opted for the former.

Change-Id: Ia51d5165f4e19f72fb04de699e657504018fb5c3

Details

Provenance
KrinkleAuthored on Apr 25 2018, 11:30 PM
Parents
rENTI75a9b603479a: Collect RUMSpeedIndex with NavigationTiming
Branches
Unknown
Tags
Unknown
References
refs/changes/35/429235/2
ChangeId
Ia51d5165f4e19f72fb04de699e657504018fb5c3