Allow changing inherited class in blocks

[maxim]

I need help deciding this. Once this is settled, portrayal will be ready to become 1.0.0.

The problem

Portrayal supports block class definition.

class ApplicationStruct
  extend Portrayal
end

class ApplicationForm < ApplicationStruct
  keyword :method
  keyword :action
end

class NewCompanyForm < ApplicationForm
  keyword :name

  keyword :address do # <--- THIS RIGHT HERE
    keyword :street
    keyword :city
    keyword :country
    keyword :postal_code
  end
end

This will create a class for you: NewCompanyForm::Address. Problem is, this class automatically inherits ApplicationForm (parent of the surrounding class). This is supposed to be a useful feature, because in most cases you want this.

Unfortunately, in this example it's undesirable, because you don't want method and action to be added to the Address subsection of the form.

I'd like to figure out the best way to avoid including method and action into Address.

Option 1

One way is to do nothing, and you just have to use regular class syntax to work around this:

class NewCompanyForm < ApplicationForm
  keyword :name
  keyword :address

  class Address < ApplicationStruct
    keyword :street
    keyword :city
    keyword :country
    keyword :postal_code
  end
end

Option 2

We can add an inherit option. If you don't provide it, it would work the same as today. If you provide it, you could do this:

  keyword :address, inherit: false do
    # address fields
  end

Or you could inherit something else:

  keyword :address, inherit: 'AnotherClass' do
    # address fields
  end

The issue with this approach is that, what if AnotherClass doesn't have extend Portrayal in it? Should portrayal detect it, and in that case extend the Address for you? Otherwise you won't be able to declare keywords in it.

Another issue is that this is just some weird extra stuff you have to learn.

Option 3

Do nothing and encourage the use of modules instead of subclassing.

class ApplicationStruct
  extend Portrayal
end

module Form
  def self.extended(base)
    base.keyword :method
    base.keyword :action
  end
end

class NewCompanyForm < ApplicationStruct
  extend Form

  keyword :address do
    # … address keywords …
  end
end

Now only NewCompanyForm gets method and action, and Address inherits ApplicationStruct.

The problem with this approach is that now you have to remember to do 2 things (inherit and extend) to make a form object.

Option 4?

Maybe there's a more elegant solution? Any suggestions are welcome.

[soveran]

I like option 1, I think it's very straightforward and reveals what is going on.

Following the example, these two would be equivalent, right?

class NewCompanyForm < ApplicationForm
  ...

  class Address
    extend Portrayal
    ...
  end
end

class NewCompanyForm < ApplicationForm
  ...

  class Address < ApplicationStruct
    ...
  end
end

[maxim]

@soveran (awesome, thanks for chiming in! 🤗)

these two would be equivalent, right?

Yes, exactly. (Edited OP for clarity.)

I like option 1, I think it's very straightforward and reveals what is going on.

Hmmm. Maybe it's an argument against the whole block feature? 🤔

Why ever do this:

class Company < ApplicationStruct 
  keyword :address do
    keyword :street
    keyword :city
  end
end

If you can always do this instead:

class Company < ApplicationStruct
  keyword :address
 
  class Address < ApplicationStruct
    keyword :street
    keyword :city
  end
end

I personally like block syntax for a couple of reasons:

1. It encourages you to keep the whole structure in one place (which I prefer). With normal classes, people will want to put them into separate files.
2. It visually ties together the keyword and the type meant for it. I don't like enforcing types in Ruby, but I do like having visual cues.

You're right that option 1 is straightforward, but what do you think about these 2 benefits? Not worth it?

[soulcutter]

My experience with RSpec is that a DSL-based inheritance and behavior was a challenge beginners to pick up and understand intuitively. It seemed to reflect a violation of The Principle of Least Astonishment. I agree that the block for would encourage people to keep the structure local, and I also prefer that in most cases like these. Still, many people will extract every class into its own file, as is their prerogative.

My instinct would be to reach for Option 1 with an extension to the keyword syntax, something like:

class Company < ApplicationStruct
  keyword :address, struct_class: 'Address'
 
  class Address < ApplicationStruct
    keyword :street
    keyword :city
  end
end

The struct_class argument could take a Class. If given as a String or Symbol it may be lazily-evaluated via Company.const_get(struct_class, _inherit = false). I waffle about the inherit behavior - I think a lot of people do not have the best understanding of Ruby constant lookup, and so I'm inclined to force people to be explicit when not looking for nested classes.

[maxim]

@soulcutter thanks for that! I believe the struct_class would be a no-op, right? Since there is no type enforcement in this lib, it'd be equivalent to:

  keyword :address # Intended to accept Address

[soulcutter]

@soulcutter thanks for that! I believe the struct_class would be a no-op, right? Since there is no type enforcement in this lib […]

Oh (facepalm) I think I got the idea from the block form's define argument. Forget about it.

[maxim]

@soulcutter No worries, it's definitely a bit weird. All the block does is declare a class named like the attribute, with no further ties to the attribute. I'll take this as another endorsement for option 1. 🙂

[ikraamg]

Since option 1 is already available, you can build option 2 for the advantages and preferences you mentioned. There is no force to use option 2 by those who prefer option 1. And given that this gem is not intended to have many of these DSL keywords, it's not going to overwhelm beginners.

[maxim]

@ikraamg Thanks for the feedback! You're right that it won't break options 1 and 3. However, I wouldn't want to build/support a feature I don't entirely endorse and recommend. If I could convince myself that the benefits outweigh the cost, then I'd go for it.