4 ms·

I totally agree with your points, but this approach has one big advantage - it's dead simple - define attributes and mapping and you're good to go.

• codesnik 4y ago
  You don't have to sacrifice that simplicity, actually. (And I insist on that simplicity being a wrong type, it'll bite users of your library basically right away, when they try to use it for anything apart from storage/serialisation) But you can just give an upgrade path! consider something like this: class Address attr_accessor :street, :city end class Person attr_accessor :address end class AddressMapper < Shale::Mapper mapped_class Address attribute :street, Shale::Type::String attribute :city, Shale::Type::String end class PersonMapper < Shale::Mapper mapped_class Person attribute :address, AddressMapper end # use like this PersonMapper.from_xml("...."); PersonMapper.to_xml(person) and then, for _dead_ simplicity, you can add another method generate_mapped_class "Person" which will define that PORO class for user for extra DRYness. API is basically the same, no repetition, but amount of rewrite with new requirements is drastically less. I'm not asking you to rewrite your library, and I probably won't write and release mine, just saying that considering future self isn't that hard. And yeah, it's a bit of a rant about ActiveRecord from user of Rails, since 2006.
  • halostatue 4y ago
    I haven’t looked at the Shale source code, but I suspect that it would not be hard to add `mapped_class` support the way you’ve described it, so that the business objects are not themselves mapper instances. At a guess, the `from_xml` probably does something like (vastly over simplified): def from_xml(xml_string) new.tap { |o| parse_xml(xml_string) do |key, value| o.__send__(:"#{key}=", value) end } end It would then be possible to change this to: def from_xml(xml_string) (mapped_class || self).new.tap { |o| parse_xml(xml_string) do |key, value| o.__send__(:"#{key}=", value) end } end This would make it easier to solve a larger problem of needing to serialize the same business object in different ways for different consumers with different levels of detail. It would also permit the construction of mappers for temporary objects that contain the details for more complex serializations that have indirect connections.
  • beerkg 4y ago
    I like it actually, using POROs (or any class for that matter) is definitely a big advantage. Maybe I implement something like that for version 2 :)
  • vidarh 4y ago
    An added advantage of this approach is that it allows clean integration w/any other mapper or library. E.g. you could define a mapping to your Sequel or ActiveRecord models, and in one go you have the ability to roundtrip between JSON/XML etc. and the ORM. To the point of rewriting: A halfway point is to drop inheritance in favour of include/extend'ing the models. If that's done cautiously, it allows for co-existing with model objects from libraries that require inheritance. That is, this: class Person include Shale::Mapper end is preferable to class Person < Shale::Mapper end
• Timothy055 4y ago
  Agreed that this is a big advantage. I’ve switched to having a separate set of serialization objects with straightforward copy constructors or mapping functions and let the serialization library do the job against those. I used to hand roll the serialization, but this is admittedly user.