format_with scope discussion

[marshall-lee]

I want to do some refactoring in Grape::Entity that will help me face #56 and #140 issues easily.

Another thing that stops me from doing it is a format_with feature. Don't ask me how it's exactly related to issues mentioned above. For now I just want to discuss some ways to improve the format_with feature.

1. Current format_with behavior is to store procs in formatters table which is copied when entity A is inherited by entity B. So each entity has its own table of formatters. I'd like to call current implementation dynamically scoped.
2. There is a way to improve it! This table of procs and the mechanism of copying is very similar to regular Ruby classes that support inheritance out of the box. We can substitute this Hash table with an anonymous formatters class and simply inherit it when the whole Entity is inherited. It will allow children formatters to refer overriden implementation (using Ruby's super) and even do mixins (with include). Implementing this will be zero cost because all that inheritance stuff will be handled by Ruby. By the way, I also call this variant dynamically scoped.
3. Make a breaking change implementing formatters lexically scoped. I mean the following behavior:

class E < Grape::Entity
  expose :created_at, format_with: :timestamp # this will raise an error because :timestamp is not yet defined
  format_with(:timestamp) { |dt| dt.iso8601 }
  expose :old_created_at, format_with: :timestamp
  format_with(:timestamp) { |dt| dt.httpdate }
  expose :created_at, format_with: :timestamp
  # Here old_created_at will be formatted with iso8601, 
  # but created_at will be formatted with httpdate

It's really breaking but it also adds flexibility as in the second variant and it's very similar to how Grape's DSL works (remember the stackable nature of params). It will also allow us to define local formatters, for example:

class E < Grape::Entity
  format_with(:timestamp) { |dt| dt.httpdate }
  with_options(format_with: :timestamp) do
    expose :old_timestamps do
      format_with(:timestamp) { |dt| dt.iso8601 }
      expose :created_at
      expose :updated_at
    end
    expose :created_at
    expose :updated_at
    expose :deleted_at
    expose :last_accesed_at
    # ...
  end
end

So no inheritance tricks needed because now formatters are lexically scoped.
Internally it can also be implemented with anonymous class so it offers the same features as the second variant but the implementation will be more tricky.

What do you think? I'm in doubt but personally I like the third variant more.

[marshall-lee]

Another reason to rethink this feature is that in #56 we decided not to just overwrite exposures but think of them as method calls. So maybe we don't need just rewrite formatters dynamically too?

[marshall-lee]

The main caveat of the third variant is that formatter should be defined before its name is used in options passed to exposure. But it's easy to catch it when upgrading because such exception will be revealed soon (during the entity class initialization).

[marshall-lee]

On the other hand, second variant makes think of formatters as helper methods and, you know, in Ruby access to methods is done using method table lookup when this method is needed.

And the stackable behavior described above is also can also be implemented with the second approach.

[dblock]

I don't see a problem with creating an anonymous class and inheriting it, although I don't see whether that improves anything. Try to make some code changes, I'd want to see the scope of changes for the user.

[marshall-lee]

I'm temporarily closing it. I tried to come up with an implementation and I don't like it. The main reason is that format_with blocks are executed in the context of an Entity instance. Maybe later I re-open this issue with fresh thoughts.