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.

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

Memoize methods that depend on instance state automatically #9

Open
dkubb opened this issue Jan 6, 2013 · 4 comments · May be fixed by #25
Open

Memoize methods that depend on instance state automatically #9

dkubb opened this issue Jan 6, 2013 · 4 comments · May be fixed by #25

Comments

@dkubb
Copy link
Owner

dkubb commented Jan 6, 2013

An immutable object's state never changes, so the #hash, #inspect and #to_s methods are idempotent. These methods should be memoized by default.

@mbj
Copy link
Collaborator

mbj commented Jan 6, 2013

I like this change.

We have to communicate methods need to be present / changed before Adamantium is included. Especially in conjunction with Equalizer.

Correct:

class Foo
  include Adamantium, Equalizer.new(:foo)
end

Incorrect:

class Foo
  include Equalizer.new(:foo), Adamantium
end

Module#include does include modules in reverse order.

@dkubb
Copy link
Owner Author

dkubb commented Jan 6, 2013

@mbj sure, the first thing we could do show examples using include on separate lines, then it's more clear the ordering of things. We can have an "advanced" section showing this one-liner, which includes Equalizer first then Adamantium.

@mbj
Copy link
Collaborator

mbj commented Jan 6, 2013

I just remember a chat conversation in #jruby, we improved performance on rack hashes with using separate String subclasses for keys that freezed contents and cachesObject#hash I lost the gist the guy created but we had around 20% speedup.

dkubb added a commit that referenced this issue Dec 16, 2013
@dkubb dkubb linked a pull request Dec 16, 2013 that will close this issue
@dkubb
Copy link
Owner Author

dkubb commented Dec 16, 2013

@mbj wdyt about #25 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants