Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idea: using for loop #59

Merged
merged 4 commits into from
Apr 8, 2019
Merged

Idea: using for loop #59

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8fc5600
moving to for loop
alexreardon Apr 1, 2019
dfe807e
adding comment
alexreardon Apr 1, 2019
d8107c9
Merge branch 'master' of github.com:alexreardon/memoize-one into usin…
alexreardon Apr 8, 2019
b6d8690
pulling input check out
alexreardon Apr 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
// @flow
export type EqualityFn = (newArgs: mixed[], lastArgs: mixed[]) => boolean;

const shallowEqual = (newValue: mixed, oldValue: mixed): boolean =>
Copy link
Owner Author

Choose a reason for hiding this comment

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

function call not required: just doing it right in the for loop

newValue === oldValue;

const simpleIsEqual: EqualityFn = (
newArgs: mixed[],
lastArgs: mixed[],
): boolean =>
newArgs.length === lastArgs.length &&
newArgs.every(
(newArg: mixed, index: number): boolean =>
Copy link
Owner Author

Choose a reason for hiding this comment

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

no longer creating a new function on every call

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not a performance concern for V8, every could be even faster!
You need a benchmark to compare with.

Copy link
Owner Author

Choose a reason for hiding this comment

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

agreed

shallowEqual(newArg, lastArgs[index]),
);
function simpleIsEqual(newArgs: mixed[], lastArgs: mixed[]): boolean {
if (newArgs.length !== lastArgs.length) {
return false;
}
// Using a for loop rather than array.every for max speed
for (let i = 0; i < newArgs.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, dont cache length - https://mrale.ph/blog/2014/12/24/array-length-caching.html

// shallow equality check
if (newArgs[i] !== lastArgs[i]) {
return false;
}
}
return true;
}

// <ResultFn: (...any[]) => mixed>
// The purpose of this typing is to ensure that the returned memoized
Expand Down