Refactoring `text_area`

proposals
1

Refers to https://github.com/lotus/helpers/pull/33 .

This starts the debate on cleaner implementations of the text_area method.

2

Here’s my initial attempt at documenting the situation on the method. Think of it as a preamble.

Considering the method:

# lib/lotus/helpers/form_helper/form_builder.rb
def text_area(name, content = nil, attributes = {})

This is the method signature. It takes a name, a piece of content which might not exist and a set of attributes which default to an empty Hash.

The following examples are part of the inline docs:

# No value
text_area :hobby

# With value
text_area :hobby, 'Football'

First of all, I feel there needs to be an example that takes a set of attributes, with or without value.

# No value
text_area :hobby, 'Football', rows: 6

# With value
text_area :hobby, rows: 6

What makes this method hard to refactor properly is the conjunction of these two possibilities. When there is a value, everything works fine; if there is no value, content takes the value of the attributes hash. This is an unpredictable, volatile method signature and it needs a better solution than the one there is now.

3

Actually i don’t think that this is a problem. It’s positional arguments, that’s how they work.

text_area :hobby, nil, rows: 6

solves the problem. And i assume most of the time the value comes from some sort of model/variable anyway:

text_area :hobby, person.hobby, rows: 6
text_area :hobby, some_hobby, rows: 6

Possible solutions:

  • Block for content
  • content via attributes hash (=> method only takes two arguments, or number/meaning of arguments is depending on presence of block. See rails content_tag helper as an example)
  • check arguments for type and decide if it’s a value or a attributes hash
  • keyword arguments
  • No change, just update documentation
  • Something missing?

And from those solutions i’d prefer the “no change” one the most.

4

Your argument is good, I like it.

I find it important that everyone reaches a common ground. If we settle on this, I can send a new PR with updated docs and the extra code removed.

PS: does this mean that content can be no longer nil? Maybe an empty string is better?

5

Just realized that i forgot the solution where content must be provided.

I’d allow nil as well. Otherwise you’d need to add that nil-check outside of the helper.