dkubb/adamantium — pull request #4 by mbj (2012-10-14 23:39:42+00:00)
Title: Do not (deep) freeze memoized values and add idempotent macro.

I thought I would push this directly to master, but after preparing commits I realized the changes could be too far. And as I wrote my initial PR I decided to revert the split and present a PR with only one behavior.

@solnic, @snusnu, @dkubb, This PR is very verbose in text to let you understand what happened in past.

The story.

I used ````Veritas::Immutable```` for a while in dm-session (former session), and mutant. As I liked to have immutable extracted dkubb/immutable was born. As immutable was only used in veritas before the freezing of memoized values was expected and proven to be good. Veritas does not really see explicit mutable data structures. Buffers, Domain Objects, etc.

The dkubb/immutable also used IceNine.deep_freeze and thus frozed all transitively reachable objects. This was first spotted as a problem when Mutant was holding class objects in ivars.
The classes where frozen so mutations could not be injected. So I began to create frozen Wrappers to defend against IceNine (IceNine does not recurse into an object when object is already frozen).

As I got bored on these wrappes I created the experimental branch in immutable that worked just like master but with no (deep) freezing memoized values.

Now I talked with @dkubb about the situartion and we ended up into two macros. ````memoize```` should just store the value returned by memoized method, and ````immutable```` that behaves exactly like memoize before.

As immutable was renamed into adamantium all my ````:git```` sources broke so I begun to create a patch that splits the behaviors. Also there is a release of adamantium, so I do have to get my change into mainline now.

Yeah, and 30 minutes in the past I finished the patch and while writing the PR text I realized the macro split is just **bad**. We force the user of the library to think about what method of memoization to use.

Providing options and not doing the right thing is always bad. And I asked my self on what rule I would drive this decision. And suddenly I realized:

**For all the methods I would have used ````idempotent :foo```` would already return frozen objects**

That is because all these objects are also including ````Adamantium```` so no need for a decision. And those who do not use Adamantium will not be prepared for freezing anyway, or would carefully be whitelisted by explicit freezing on return.

And as far as I know veritas, where the code originates from, all values that reach Veritas::Immutable#store_memory are already frozen. The same for mutant, session etc.

From this observation, pls accept my change. Sure Im also open for discussion and hopefully my point can be undestood. Catch me in IRC in case you have questions.

## Previous PR

The previous behavior did have unexpected results when a explicit mutable object is reachable transitively.

What this changes:

* Split ````memoize```` macro into a more permissive ````memoize```` and a
stricter ````idempotent````.

````memoize :foo```` creates a memoized method returning the original methods result unmodified.
````idempotent :foo```` creates a memoized method returning the same like a memoized method would but frezes it.

* Remove the use of ````IceNine.deep_freeze````. Through the concept is really
nice debugging can be very very hard when it accidentally freezes a
mutable domain object. Also its use in .new did deep freeze
regardless whether idempotent or memoize was used.

* Assert memoized and immutable methods have an arity of zero. Query
methods with nonzero arguments are per definition not idempotent.

* As ice nine is not used anymore the dependency was also dropped.

What I dislike:

* We now have two options to shot ourselves into our feed. Maybe we do not need the immutable macro at all, as the creator of the object should be reponsible on freezing or not freezing it.
* It does not use ````IceNine.deep_freeze```` and I would really like to find a nice use case for it outside the freezing of Arrays / Hashes assigned to constants.

@dkubb I'd really opt for removing the immutable behavior, but for this I need your okay as the creation of the immutable macro was our discussion result.