DSL syntax

[jonleighton]

@avdi has proposed a DSL syntax: https://gist.github.com/3757036

I initially resisted adding a DSL to FC because:

• I didn't want to obscure "what lies beneath" too much
• I don't find the existing syntax too hard to swallow
• I didn't want people to think that FC is so different from "ordinary" controllers when it isn't.

However, I am coming around to the idea:

• Everyone loves a DSL and if this makes some people keener to use FC that's good with me
• It can be purely optional and we can make "what lies beneath" clear through documentation
• It can allow new patterns of reuse, such as "full controller inheritance" where every action in one controller is inherited from every action in another. This could be useful for fast prototyping, for example.

But let's go one step at a time and get something working, try it out in real applications, then iterate.

I like Avdi's syntax, but have some comments:

• BenefitsController = FocusedController::Controller.new do is a bit "out there" I think, and using techniques that people might find confusing.
• There needs to be a syntax for inheriting actions. For example I usually make create inherit new and update inherit edit. So here's my stab at it:

FocusedController.define :posts do
  common do
    before_filter :authorize
  end

  action :index do
    expose(:posts) { Post.all }
  end

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

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

  shared :find do
    expose(:post) { Post.find params[:id] }
  end

  action :show, extends: :find
  action :edit, extends: :find

  action :update, extends: :edit do
    def call
      # ...
    end
  end

  action :destroy, extends: :find do
    def call
      post.destroy
      redirect_to posts_path
    end
  end
end

I am not sure about the shared thing. An alternative is:

  group do
    expose(:post) { Post.find params[:id] }

    action :show
    action :edit

    action :update, extends: :edit do
      def call
        # ...
      end
    end

    action :destroy do
      def call
        post.destroy
        redirect_to posts_path
      end
    end
  end

Not sure about that either.

Alternatively edit, update and destroy could technically just extend show, but that feels a bit wrong conceptually although it seems fine pragmatically and is less verbose.

[bjeanes]

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.

[h-lame]

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.

[jonleighton]

@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?)

[h-lame]

@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.

[h-lame]

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.

[b4mboo]

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.

[avdi]

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.

[avdi]

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

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

[jonleighton]

@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...