Adding memoization - Githubissues

gpbl / denormalizr

Denormalize data normalized with normalizr

https://www.npmjs.com/package/denormalizr

MIT License

228 stars 24 forks source link

Adding memoization #20

Open yachaka opened 8 years ago

yachaka commented 8 years ago

Hello,

This is the PR discussed in issue #10. API follows what has been discussed : { memoized: true } to activate memoization.

I've been rather busy so I just added few tests and the overall code could be refactored in some points somehow but it does the job.

Needs the docs to be partly rewritten though to explain how to use/the advantages of using memoization.

Note: the cache is for now global. In the future. It would be necessary to take is as argument to allow the user to maintain a cache for each set of entity he might be using.

gpbl commented 8 years ago

Awesome @yachaka, thanks a lot! Please give some some time to review the changes.

gpbl commented 8 years ago

@yachaka Thanks again for your PR. Yes, I see the need of a refactoring to avoid some code dupes... If you are have some time for a code review, we can work on this together, otherwise I can try updating the branch myself in the next days.

clessg commented 8 years ago

@yachaka Thanks for this. Really looking forward to this change. 🎉

yachaka commented 8 years ago

@clessg Thanks for the feedback ! Really appreciated ! :) @gpbl I can look onto refactoring a little next week, if no one does !

gpbl commented 8 years ago

Sorry, I've broken the PR fixing up some code in master :( The valuable code is still visible and reusable from the diff.

yachaka commented 8 years ago

I will be able to look into refactoring along your comments this friday. ;)

diegohaz commented 7 years ago

Hey guys. Any updates on this?

yachaka commented 7 years ago

@diegohaz I've been a lot busy lately (just started a new job), but I will into refactoring this week (promess 😸). For now, you can use my fork (yachaka/denormalizr) ; it works with memoization using { memoized: true } as the first argument.

coveralls commented 7 years ago

Coverage decreased (-8.4%) to 91.603% when pulling 8238d5057b74e9ded61943d02cf51f3aab0f32af on yachaka:master into 08ede86a2c472cb1d6ad54e3f60455e2ecf0d7fc on gpbl:master.

coveralls commented 7 years ago

Coverage decreased (-8.4%) to 91.603% when pulling ee2d493457ace530ecb48d6ffae49b0ff13e8859 on yachaka:master into 08ede86a2c472cb1d6ad54e3f60455e2ecf0d7fc on gpbl:master.

yachaka commented 7 years ago

@gpbl Updated README, added some code comment. I didn't refactor the denormalize entry point function ; I think it is more readable this way - splitted into 2 functions. In term of performance, it also eliminates a check. Please review 👍

coveralls commented 7 years ago

Coverage decreased (-8.4%) to 91.603% when pulling 95b72837cc005328daf7c078c74cd743842a915a on yachaka:master into 08ede86a2c472cb1d6ad54e3f60455e2ecf0d7fc on gpbl:master.

yachaka commented 7 years ago

@gpbl Sup ?