DSL syntax about focused_controller HOT 9 OPEN

What about the following for inheritance:

action :create => :new do
  def call
    post.update_attributes params[:post]
    redirect_to posts_path
  end
end

It runs the risk of being confused with Rake/Capistrano-esque idioms in that a developer could mistake it for "run :new before running :create", but I think eliminating :extends makes the DSL feel a bit more domain-specific.

I am not usually one who pushes for more DSLs over clear code, but I find the :extends jarring in this context.

from focused_controller.

I'm conflicted about this. Sure it's neat, but I think a DSL like this obscure the underlying details and make things feel more like magic. I think it'll make it harder to get the test benefits (what are my classes called?!?). I'm not opposed to it, just I'd want it to be optional.

That said, if we're obscuring ruby to make a DSL I don't think it goes far enough. Why isn't the action block more like this:

action :create do
  post.update_attributes params[:post]
  redirect_to posts_path
end

Why do I have to do def call ... end at all? It sticks out as a bit boiler-platey and, for me, the purpose of a DSL is to get rid of as much boilerplate as possible. Implementation-wise I know it's because we just eval the block onto the class, but that just seems lazy.

Taking it further (and I've thought this through much less), when you need to do extra stuff in the action (expose, before_filters, etc...) you could just chain it on the return. Something like:

(action :new).exposes(:post) { Post.find params[:id] }

Although I'm less convinced by that than I am the def call-less variant. Particularly because it makes it very weird to define method-y filters.

from focused_controller.

@bjeanes The problem for me with :create => :new is the direction of the arrow. It goes the other direction that for class Create < New

@h-lame I don't think it would work to put the contents of call in the action block. It's mixing the class-level scope with the method-level scope. Consider if we have another method in the action:

action :create do
  post.update_attributes params[:post]
  redirect_to posts_path

  def foo
    # ...
  end
end

That just looks weird to me. Not to mention there would be problems with implementing it (when does it get evaluated?)

from focused_controller.

@jonleighton Sure, that wouldn't work, but some other construct could. I'm thinking that the block passed to action would always be the contents of call and there'd be some other way for defining these other methods (perhaps just an alias for define_method, if action returns a Class instance). It's definitely very warty though. I suppose I just don't think the DSL is worth it as it is, but making it go far enough makes it super weird.

from focused_controller.

Anyway, until I've used it in anger (with or without any DSL), my comments should probably be taken with the saltiest pinch of salty salt.

from focused_controller.

I really love the idea of introducing a DSL. Making it optional is a big plus. Especially since I'm not quite sure how to solve the "what are my methods called" issue, when it comes to testing.

As for sharing code between actions: I like the group syntax. However, again, the example using :find is probably less confusing if I wanted to test the shared code.

from focused_controller.

I'm not convinced how much need there is for inheritance. It seems to me any shared-functionality scenario is handled at least, if not more, cleanly by factoring out a module.

module CreationalAction
  # ...
end

action :new do
  include CreationalAction
  # ...
end

action :create do
  include CreationalAction
  # ...
end

However, if you do have a keyword argument for inheritance I'd prefer :inherits, since "extend" has a specific meaning in ruby.

from focused_controller.

Either that or just have the parent be a positional argument, similar to how Class.new(Parent) works.

action :create(:new) do
  # ...
end

from focused_controller.

@avdi the problem with using a module is that you can't do stuff like expose in there. in theory we could provide a way of constructing "modules" that can use expose and other declarations, but it's still making things more verbose. For example compare:

module Instantiator
  expose(:post) { Post.new }
end

action :new do
  include Instaniator
  # ...
end

action :create do
  include Instantiator
  # ...
end

with:

action :new do
  expose(:post) { Post.new }
  # ...
end

action :create, inherits: :new do
  # ..
end

The latter is much more favourable to me...

from focused_controller.