dkubb / adamantium

Create immutable objects
MIT License
373 stars 13 forks source link

Handle immutable versions of base classes #2

Closed misfo closed 11 years ago

misfo commented 11 years ago

including Immutable in a subclass of Hash, for instance, doesn't give the intended behavior for methods that return new Hashes:

irb> class Thing < Hash
irb>   include Immutable
irb>   end
=> Thing
irb> t = Thing.new
=> {}
irb> t.frozen?
=> true
irb> t2 = t.merge(:a => 1)
=> {:a=>1}
irb> t2.frozen?
=> false

Would it make sense to include immutable base classes (e.g. Immutable::Hash) that include Immutable and override methods like these that need to be handled?

mbj commented 11 years ago

I normally do not subclass primitives like Hash, String etc. So I cannot see the use case. @dkubb what is your opinion?

dkubb commented 11 years ago

@mbj yeah, I'm not a big fan of subclassing primitive objects.

There's a few reasons for this. One is Primitive Obsession. I think if you have some concept you're modelling, usually primitives aren't a good fit. There's lots of good info about this, so I won't try to echo what's already public.

@solnic wrote a good article on this that we actually discovered while working on this and related projects: http://solnic.eu/2012/06/25/get-rid-of-that-code-smell-primitive-obsession.html

Another is that when you subclass an object, I like to make sure that my object can be substituted for instances of the parent class. Usually this is called the Liskov Substitutability Principle.

The reason I like to follow this is that like the idea of defining interfaces that my objects have to implement, and that as long as I don't break the input/output constraints on the subclass methods, I can duck type things and there's a good chance that it'll just work.

dkubb commented 11 years ago

One problem that you run into when subclassing objects is that you change the implementation of one mutator, but you forget about changing the interface of related methods, and then you have behavior that is inconsistent.

For example, imagine subclassing Array and you override #<< to do something different. You've forgotten about #push, #concat, and maybe #unshift, #pop, #shift even. Someone uses your object, checks that obj.kind_of?(Array), and then uses one of those other methods which doesn't do the right thing from your pov, but mutates the internal state of the object.

What I think is a better approach is to favor composition over inheritance. You write an object that "has a" primitive object under the hood, but exposes a minimal, purposeful, public interface. You can control all the inputs and outputs, and have a much better guarantee about how it will perform and what possible states the object can be in. With a primitive subclass you can never be sure, new versions of Ruby can change or add new things which bypass your intentions.

misfo commented 11 years ago

I'll admit I've never subclassed a primitive in Ruby before, so this was kind of a bogus issue to begin with. But I'm not convinced it's a bad idea either.

In languages like Clojure it's idiomatic to represent associative data as... associative data. All the generic map manipulation functions work just as well on "records" (which are roughly equivalent to the subclassed Hashes mentioned here).

I much prefer that approach to hiding associative data behind accessor methods in an object. But the answer may be that primitive obsession is a "code smell" in Ruby simply because its not idiomatic...

Sent from my phone

On Oct 10, 2012, at 11:01 PM, Dan Kubb notifications@github.com wrote:

One problem that you run into when subclassing objects is that you change the implementation of one mutator, but you forget about changing the interface of related methods, and then you have behavior that is inconsistent.

For example, imagine subclassing Array and you override #<< to do something different. You've forgotten about #push, #concat, and maybe #unshift, #pop, #shift even. Someone uses your object, checks that obj.kind_of?(Array), and then uses one of those other methods which doesn't do the right thing from your pov, but mutates the internal state of the object.

What I think is a better approach is to favor composition over inheritance. You write an object that "has a" primitive object under the hood, but exposes a minimal, purposeful, public interface. You can control all the inputs and outputs, and have a much better guarantee about how it will perform and what possible states the object can be in. With a primitive subclass you can never be sure, new versions of Ruby can change or add new things which bypass your intentions.

— Reply to this email directly or view it on GitHub.

dkubb commented 11 years ago

Yeah in Clojure that's definitely the approach that Rich Hickey has decided is better for a lisp-like language, which makes sense.

I think it doesn't fit so well in object oriented languages though. People either end up scattering the operations that work on the data throughout the code or they monkey patch the core classes, which is probably worse. You rarely see people subclass primitives properly, it's really hard to do perfectly. I've done it for some internal DM1 libs a few years ago and it took me a month to get a simple Array-like object to production level (see LazyArray in dm-core).