Closed groyoh closed 9 years ago
It's great that you're working on this. @plexus had a couple of suggestions what the derive_mapper_from_single_object
method would be named, and I and one other person agreed that dervie_mapper_from_item
is the best. What do you think about this?
My first thoughts was that item could be mistaken for an "item inside a collection". But I admit that item indeed sound better and is shorter. If you all agreed about it, I don't mind changing it to item :wink:
I agree item
is a good name.
When working on mapper_for
, I stumbled on a few issues:
namespace
option?# @options[:namespace] == Mappers
class Thing; end
mapper_for Thing, ThingMapper
derive_mapper_from_object(Thing.new) # => ThingMapper or Mappers::ThingMapper?
mapper_for :thing, ThingMapper
derive_mapper_from_object(Thing.new) # => ThingMapper
derive_mapper_from_object(Stuffs::Thing.new) # => ThingMapper or continue look up?
class Widget < Thing; end
derive_mapper_from_object(Thing.new) # => ThingMapper
derive_mapper_from_object(Widget.new) # => ThingMapper or continue look up?
What do you guys think about it?
What I had in mind was to simply leverage Ruby's "case equality", ===
. When used on a class that gives you "instance of", when used on a symbol or string it's simple equality, when used on a lambda it invokes the lambda
Should we use the namespace option?
no
When using symbol, should we consider the class name and module or just the class name?
Neither. Symbols would just be for "fixed" responses where you want to invoke a certain mapper without specific data to serialize
A common use case is having a "home page" which only contains links as a starting point to traverse the API
mapper_for :home, HomeMapper
derive_mapper_from_object(:home) #=> HomeMapper
When a mapping is defined for a class, should we derive this mapper from only instances of this class or also instances of subclass?
===
also matches subclasses, so that's your answer :)
class Foo
end
class Bar < Foo
end
Foo === Bar.new # => true
Nice, thanks! I did not know that ===
invokes a lambda. And I did not thought about symbols this way!
Everything makes sense now! :smile:
PR ready to be reviewed! :smile:
Made some nice refactoring!
@groyoh I left some nitpicks, really good job overall :+1:
I made the changes according to your comments. @plexus I removed the errors classes. Whenever you decide whether we should keep custom errors classes or not, let me know.
Based on the discussions in #72 and #76,
derive_mapper_from_collection
andderive_mapper_from_single_object
can now be overriden independently andderive_mapper_from_object
will call either of these two depending on the object type.The
mapper_for
option is now available too.