Skip to content

Small improvement to memoize fn #10

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

Open

mbonaci wants to merge 1 commit into reactjs:master

from mbonaci:patch-1

Open

Small improvement to memoize fn #10

mbonaci wants to merge 1 commit into reactjs:master from mbonaci:patch-1

Conversation

Copy link

@mbonaci mbonaci commented Oct 29, 2016

No description provided.


 Small improvement to memoize fn

d5548f4

Copy link

dmitriz commented Dec 7, 2016

Why is this an improvement?

Copy link

Author

mbonaci commented Dec 19, 2016

Isn't it a bit cleaner?

Copy link

dmitriz commented Dec 19, 2016

Not to me. If arg is cached and nothing is to do, I'd like to get it out of my way first. That makes it more scalable as all changes would go below.

Copy link

ulpian commented Apr 6, 2018

Why would scalability be an issue with this improvement?

Copy link

huyz commented Apr 7, 2018

The original code requires less cognitive load when reading. Once you get one condition out of the way with an early return, you avoid having to keep two conditions simultaneously in your head while reading the rest of the function (and making sure you understand it and it is bug-free). So "cleaner" (i.e. less repetitive) doesn't imply "easier to read" and "easier to debug".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment