standard-library / microform

Tiny form object pattern for Rails
MIT License
0 stars 0 forks source link

Shared functionality: ApplicationForm? Mixin? Boilerplate? #1

Open jackjennings opened 7 years ago

jackjennings commented 7 years ago

Should microform also have an "install" generator that adds an ApplicationForm class to your project?

Or, taking a step back, how can some of the generic functionality like the respond_to? and other delegation method. This could either be a parent class, or a mix-in module, or simply repeated in each form class.

The next question is, if functionality is shared in either of those way, does that mean that the instance variable needs to get more generic:

def initialize project
  @project = project
end
# vs
def initialize record
  @record = record
end

Or, is there some way that the instance variable is configured. This could be too clever:

class ProjectForm
  include Micrform::Form[:project]
end

Or, the instance variable could be inferred from the form name, e.g. ProjectForm assumes there will be a @project instance variable assigned in the initializer.

Do these "solutions" jump trough too many hoops to just avoid renaming the attribute to the more generic record?

jackjennings commented 7 years ago

If we're taking a cue from Pundit, they're using record and installing an ApplicationPolicy: https://github.com/elabs/pundit/blob/master/lib/generators/pundit/install/templates/application_policy.rb

ghost commented 7 years ago

Hmm, yeah, an ApplicationForm and generic @record seems useful.

def initialize record
  @record = record
end

Repeating this would be repetitive, but I'm wondering if it would be the most readable to people? Hmm.

jackjennings commented 7 years ago

record record record

Yeah, I like when variable names are specific, but willing to make concessions if we can hide some of the implementation details.

It could also be "subject", which at least points back towards the name of the form class for context.

Though I'm also aware of keeping consistency with previous artwork and the fact that pundit uses "record"…

jackjennings commented 7 years ago

I was wondering if maybe this takes the form of some modules that import class methods:

class ProjectForm
  extend Microform::Delegator

  attr_reader :project
  delegates_to :project

  # ...
end

Then that delegates_to class method defines the method_missing, respond_to? etc, passing them to the appropriately-named variable.

jackjennings commented 7 years ago

Could open the door for other mixins as well… and make some functionality opt-in.

jackjennings commented 7 years ago

I guess it does make me uncomfortable that it pushes functionality into the gem, instead of being "owned" by the project. In the case that there is a concrete ApplicationForm, at least that can be generated and live in the project instead of reaching into the library code to inject functionality like the above example.

jackjennings commented 7 years ago

I'm leaning towards an ApplicationForm, after reading the original post in this PR about standardizing to Application* classes: https://github.com/rails/rails/pull/28243

But I'm not sure what gets included there. A blank class? A class that somehow has the method_missing etc defined, or that defines a class method to do so in child forms. Would push for the simplest option, but I'm not sure yet what that is.

jackjennings commented 7 years ago

ApplicationForm could be a SimpleDelegator, and just sidestep any reference to the object at all:

class ApplicationForm < SimpleDelegator
end

class ProjectForm < ApplicationForm
  def submit changeset
    update changeset
  end
end

In effect that's the same that we're doing with method_missing now, but feels weird on a gut level. Seems like there might be some strangeness based on the readme of this library I just found: https://github.com/stevenharman/dumb_delegator