Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Reduce unnecessary repetition of loops #2982

Merged
merged 1 commit into from
Apr 21, 2020

Conversation

nicholasjpaterno
Copy link
Contributor

@nicholasjpaterno nicholasjpaterno commented Apr 20, 2020

No need to loop through with filter then reduce. Since filter can be implemented with a reduce we can eliminate the need to loop through expr.split(".")'s output twice.

Object.keys(obj) is actually a O(n) operation. Lets do this once and save the output.
We can also eliminate the need to map (O(n)) this before passing the result to Math.max which is a O(n) operation as well. If we just perform the operation in one reduce statement.

Nothing imperative here, just low hanging fruit.

Copy link
Contributor

@haltman-at haltman-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me once the build is fixed!

No need to loop through to `filter` then `reduce`.  SInce filter can be implemented with a reduce we can reduce the need to loop through `expr.split(".")`'s output twice.

`Object.keys(obj)` is actually a `O(n)` operation.  Lets do this once and save the output.
We can also eliminate the need to map (`O(n)`) this before passing the result to Math.max which is a `O(n)` operation as well. If we just perform the operation in one `reduce` statement.

Nothing imperative here, just low hanging fruit.
@nicholasjpaterno nicholasjpaterno force-pushed the nicholasjpaterno-patch-printer.js branch from 894c47c to 78949db Compare April 21, 2020 03:07
Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@nicholasjpaterno nicholasjpaterno merged commit 6405930 into develop Apr 21, 2020
@nicholasjpaterno nicholasjpaterno deleted the nicholasjpaterno-patch-printer.js branch April 21, 2020 14:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants