memoist
memoist copied to clipboard
Raises exception instead of emitting warning.
The message tries to guide the developer on how to workaround the issue.
It's better to fail rather than skip memoization which can lead to bugs.
@paneq thanks for the work.
Any idea why it fails on the old rubies?
I like the idea of giving more information, but why change from a warning to an exception?
I'm not keen to make any breaking changes.
Any idea why it fails on the old rubies?
nope
why change from a warning to an exception?
- noticing a warning might be hard (foreman or docker with tons of other output)
- noticing a warning does not automatically lead to fixing it by a developer who does not understand where it is coming from and what to do
- memoizing is not always used for performance speedup. In our cases it was used for caching the calls which return random results, which should be reused. So essentially the current design leads to hidden bugs.
I'm not keen to make any breaking changes.
Assuming developers don't add memoize
in runtime and they eager load entire codebase (ie rails app in testing) they should get this exception (with instructions on how to fix it after upgrade) when running tests.
They can even apply the recommended changes in the code using older gem version, and then upgrade to a newer version which won't crash.
I am not particularly interested in supporting developers who:
- don't test
- their tests don't load entire codebase
- dynamically create subclasses with memoization (not working in fact) in runtime
On the other hand I prefer to raise an exception, when a guaranteed bug occurs, for the rest of the developers out there so that they can notice it, the moment they introduced it and fix accordingly.
Up to you.
I spent plenty of time looking at code with memoize
in front of it, thinking that it memoizes the value and wondering what is wrong and where a certain bug comes from, only to discover that the value is not memoized at all.
So this is where my recommendation comes from.
Test fails on Ruby < 2.1 due def
behavior change (https://bugs.ruby-lang.org/issues/3753). Until Ruby 2.1 def
was not returning defined method name as symbol, MRI was returning nil.
Tests needs to be fixed to call memoize :method_name
after method definition.
@barthez https://www.ruby-lang.org/en/news/2017/04/01/support-of-ruby-2-1-has-ended/
This gem should drop older Rubies eventually because eventually supporting them will be detrimental to forward progress. But if supporting them is not yet detrimental, then there is little reason to break the support.
@matthewrudy @pboling Fixed Ruby 2.1 support, build is green now.
Do you think it's ok to introduce breaking changes even though the gem has not reached 1.0 yet?
@matthewrudy @pboling What are your thoughts on this guys?
@matthewrudy @pirj I love this change. I prefer noisy failure, early, and as often as possible when things are legit broken.
Ping.
@matthewrudy what do you think of this change?
Just a friendly ping one year later :)
@paneq @pirj Matthew, the owner of this repo, died in 2019. I recently found out. I have created a new org in his memory, called memoist
. I'm adding you to it (don't have to accept if you'd rather not).
Matthew's brother, @sebjacobs, maintained the repo for a while through 2020, but it isn't clear if he is still involved in open source at this point.
Thanks, @pboling. Sad to hear that about Matthew.
Unfortunately enough, I'm out of additional capacity ATM to maintain memoist
.
I copied Matthew's repos to a new @memoist org, but it is looking like they would have to be hard forks into a new gem namespace to continue.
In the meantime I am switching to https://github.com/makandra/memoized
FYI: Added this alert to the new memoist repo
[!IMPORTANT]
Recommendation
Consider using MemoWise instead, as it is maintained, fully tested, provides thread safety guarantees, and is much, much faster.
Other Alternatives
In case you need a tool with this feature set that is currently maintained, check out:
- https://github.com/makandra/memoized
- https://github.com/honzasterba/memoist
- https://github.com/tycooon/memery
[!TIP]
Seriously though, read the important note above.
[!WARNING]
If you must continue - be aware that this is unmaintained software.