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

Using multiple Representers to decorate an class #114

Open
barttenbrinke opened this issue Sep 24, 2014 · 7 comments
Open

Using multiple Representers to decorate an class #114

barttenbrinke opened this issue Sep 24, 2014 · 7 comments

Comments

@barttenbrinke
Copy link

As discussed here: https://groups.google.com/forum/#!msg/roar-talk/a0_8zSKuMZw/k8bTzYAelJ4J

Extending with multiple representers would be very handy when working with a Data/Context/Interaction setup.

I am trying to do the following:

 class Person < ActiveRecord::Base
 end
  class PersonDecorator
    include Roar::Representer::JSON
    property :id
  end
  class PrivatePersonDecorator
    include Roar::Representer::JSON
    property :secret_name
  end

In the controller:

  person = Person.find(params[:id])

  person.extend(PersonDecorator)
  person.extend(PrivatePersonDecorator) if current_user.admin?

  present person

Currently I worked around this by writing an initializer that changes https://github.com/apotonick/representable/blob/master/lib/representable.rb#L80-L83 to do inherit on extend if there are representable_attrs in the Object. I also needed to make the representable_attrs public for that.

What whould I need to write in order to get a pull request accepted for this?

@chussenot
Copy link

It's a great idea... And i like it ! 👍

Do you know that you can pass options in to_hash or to_json methods ?

 options = {count: 27}
 test.extend(TestRepresenter).to_hash(options.merge(
  { base_url: "#{some_domain}",
    current_user: current_user
  }
))
module TestRepresenter
  include Representable::JSON
  include Roar::JSON::HAL

  property :id
  property :name
  property :results, getter: lambda { |args| check_result(args[:current_user]) }

  link :self do |opts|
    "#{opts[:base_url]}/tests/#{id}"
  end

  private
  def check_result(current_user)
    # Do want you want with current_user
  end
end

@barttenbrinke
Copy link
Author

When I was trying to extend Roar, I discovered that a lot of the development is in flux. Also the way that attributes are pushed around using Roar & Representable makes it very hard to extend on it. The entire thing felled very over engineered for my purpose. As I needed more flexibility in my implementation anyway, that I decided to rebuild my own DSL using a Roar like syntax. Surprisingly it took about 250 lines of code to do this, so I'd recommend to go this path if Roar feels to constrained for your intended purpose.

@apotonick
Copy link
Member

@barttenbrinke I've heard that several times and my apologies for providing gems that suit more than one client's requirements. I'm totally fine when people copy the concept of a representer and implement it with their own code.

Roar and Representable do not pass stuff around - Roar uses Representable, what's wrong with that?

You're probably doing JSON, only, this is why you end up with that little code. It needs some architecting to have a generic interface for all formats. This API has only changed slightly in the past years so I can't really understand what you mean with "in flux".

Roar has changed recently as I simplified Hypermedia, but that's the whole purpose of Open Source to keep constantly moving. 😁

I absolutely disagree in one thing, though. Roar is absolutely not constrained - the opposite is the case: You have a wide variety of ways to deserialize and render objects and hooking into that process and this is exactly what makes it look complicated. Of course, you only want a subset of this and now your code is leaner because it doesn't implement the entire API. I'd love to simplify, why don't you hit me up next time when you are having trouble understanding my madness?

@chussenot Yepp, that works exactly the way you put it.

@apotonick
Copy link
Member

Honestly, I don't get it. People use my gems, find the idea good but then rewrite it because "the original version is over-engineered." They usually end up with a subset of functionality that fits exactly their requirements. They then tell me that they "implemented it less complicated and with less code". What are they trying to say? That everyone should stop using frameworks and write it themselves because it's "less code"? Please, enlighten me what those comments are about.

@barttenbrinke
Copy link
Author

Hey @apotonick , I'm really sorry I didn't explain myself well enough in my post.
I really like Roar/Representable and it was what got me working on Hypermedia stuff in the first place.
I am currently working on a project on which I had to combine Roar with a very specific type of DCI, and in the end I was writing so much patch code, that I decided to not use Roar/Representable anymore for this project.

What I meant with over-engineerd:

  • Roar does a lot of work to hide its methods and attributes and to make sure that the roar attributes don't conflict with other parts of the existing models. This is very nice, but this also makes it very hard to extend multiple decorators, which is something I needed in my DCI setup. When you look at https://github.com/apotonick/representable/blob/master/lib/representable.rb, it sets up both class & module attribute inheritance by using inherit & extend on included, combined with uber and a representable_attrs hash, which is secretly an object. Also some of the logic resides in Roar and some of it in Representable. This is probably needed to supporting the extend option, but at a very high cost.

What I meant with in flux:

  • The code in the Roar master and Representable master on September 24 differed quite a lot from the latest releases, making it very hard for me to create a sane patch / pull request. Again the part of attribute inheritance was refactored in both masters. The release of the Beta 2 days ago will help a lot there.

What I meant with Roar is constrained:
For a DCI setup I was missing the following items:

  • Assign multiple decorators/roles to a single object
  • Authorization on attribute level (for both reading and writing).
  • Excluding attributes from an object

For all of these points I found no easy way to patch Roar/Representable to do this.

My advice to @chussenot should have been clearer: If you are trying to do a DCI with HAL, then Roar/Representable might not be the best fit at this point. Because a DCI will probably entail loads of custom logic, which is very specific for your app, it might be a good idea to build it yourselves. I did that and by implementing just the features I needed, I was able to build something pretty lean that looks a lot like the Roar Decorator syntax, which also does custom magic that I need in my specific use case.

@apotonick
Copy link
Member

Thanks @barttenbrinke sorry I had a shit day yesterday and might have over-reacted a bit. ❤️

Great points - I start to understand how you use Roar with DCI and how that might interfere with each other.

  1. I really try to keep the "inheritance" logic at a minimum, you know, stuff like the extended hook. The way the representable_attrs object is inherited, again, tries to be as simple as possible. You have to keep in mind that when including modules into modules and inheriting classes and so on, the representable_attrs has to be cloned: and not only the hash, everything in that hash and every Definition, and so on, otherwise you might mess up your original definitions in a subclass.

    I was planning to document how that works in my book. Anyway, this is all representable and shouldn't bother you in Roar?!

  2. My code is always in flux haha, I know it's hard to keep up! Appreciate your interest, and when I say "email me" I really mean so: I am always happy to explain stuff or redesign if you come up with good ideas.

  3. The "problem" with multiple modules should be tackled, now that I start to understand how you use it. (BTW, I'd love to get a brief introduction about your DCI setup, e.g. a gist)
    Authorization and excluding on attribute level is way simpler now with skip_render and skip_parse. I would LOVE to work on a better DSL for that, internally we can surely use the new :skip_* filters!!!

Thanks man!

@barttenbrinke
Copy link
Author

@apotonick See email :)

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

No branches or pull requests

3 participants