grape-entity icon indicating copy to clipboard operation
grape-entity copied to clipboard

support dynamic attributes & merging collection values

Open danhodge opened this issue 10 years ago • 5 comments

This is the start of my proposed solution to the problem I outlined on the mailing list here. It doesn't add general purpose dynamic attribute, it just allows you to evaluate the 'parent' attribute of a nested attribute structure dynamically through a hacky naming convention (__ prefix on the attribute name). I would have rather used a block to evaluate the value of the dynamic attribute, but I wasn't sure what all of the the implications of creating a NestingExposure were.

danhodge avatar Sep 15 '15 02:09 danhodge

I am personally not a huge fan of naming conventions. There should be an explicit way to tackle this. Try to propose a cleaner DSL? Maybe something as simple as an expose that doesn't take a name? What if the following exposed each field in the hash?

expose do
 { x: 1, y: 2 }
end

dblock avatar Sep 15 '15 11:09 dblock

Hello!

This feature seems good, but:

a hacky naming convention (__ prefix on the attribute name).

Please don't!

It'll be better to implement it in terms of :as option:

expose :by_date, as: -> (obj, opts) { obj.date } do
  expose :first_name
  expose :last_name
end

Also the question is how it will work with :only and :except options?

marshall-lee avatar Sep 15 '15 11:09 marshall-lee

And:

merging collection values

What is this? I don't see any merging in your code.

marshall-lee avatar Sep 15 '15 11:09 marshall-lee

I only used the naming convention for prototyping purposes, I like the as: <lambda> approach much better. I'll switch to that approach. As for the merge, it happens here. Instead of mapping over the objects in the collection, it reduces them into a single Hash by merging them together. Perhaps reduce is a better term for what its doing?

danhodge avatar Sep 15 '15 13:09 danhodge

@marshall-lee Does the as: syntax let you merge onto root at set of dynamic keys, doesn't look like that or am I missing something?

Can you comment on what you think about the key-less extension (expose with just a block)?

dblock avatar Sep 15 '15 13:09 dblock