Limit use of conditional modifiers to short, simple cases.

[MatheusRich]

This PR changes the guideline of "Avoiding conditional modifiers", which I believe lacked nuance.

This was product of a group discussion, and we thought it was good to introduce our reasoning as part of the guideline. That way, there's a new markdown file with the reasoning and examples. This might be a new pattern for the guides.

[MatheusRich]

This renders as

[rdnewmanbot]

I'm glad we're modeling this!

[rdnewmanbot]

Thank you!!!

[JoelQ]

The examples you give all seem to be pre-conditions and invariants. Maybe we just want allow this style for guard clauses? How do we feel about inline conditions in the middle of a method body?

[JoelQ]

This is especially true with unless since you an introduce a DeMorgan bug

[JoelQ]

This seems to be moving towards the Triangle of Separation style of code

[JoelQ]

I think this section is really about what is the "main action" of the method, and what is the edge-case blocked by the guard close. It seems less about formality and more about verifying invariants?

[MatheusRich]

I'd say both conditions can be equally valid, which is highlighted in the if/else example? The writer has the power to bring focus to particular branch by not using the guard. Maybe that's what I was trying to communicate with formal/informal.

[JoelQ]

If we allow inline conditions in the middle of the body, how do we feel about the style that attempts to "flatten" a conditional to fewer lines like:

def can_edit_admin_post?
  return owner? unless admin?
  true
end

that often ends up more complex than the explicit if/else. The example above is re-implementing boolean ||

def can_edit_admin_post?
  owner? || admin?
end

https://thoughtbot.com/blog/back-to-basics-booleans#multiple-variables

[JoelQ]

This guideline intersects with some other topics too:

• How do we feel about if without a corresponding else (now it's implicit, possibly nil)?
• How do we feel about early returns?

[MatheusRich]

@JoelQ it seems to me that inside a method body those would behave the same?

def brew_coffee
  boil_water
  add_coffee_grounds
  pour_water
  stir

  add_sugar if sugar?
  add_cream if cream?

  serve
end

[vburzynski]

suggestion: I think the file name should match the link label

• when they match, it lowers cognitive cost of coming up with both the title and filename
• it's easier to locate the correct file when the filename matches the title
• using the title as the filename is more unambiguous and natural to use.
• would facilitate consistency in naming files
• The file name is more likely to be unique, making it easier to generate links in markdown editors like Obsidian automatically suggest pages to link to
• Differentiates between other files that may also speak about conditional modifiers
• This is similar to what happens on Wikis where the title and file name are the same
• Using a propositional title (at least for those notes where the title forms a sentence or independent clause; such as statements, assertions, declaratives, advice, claims, theories, and/or imperatives) is useful for forming interconnections between pages because the filename/title forms a concept handle. Concept handles can then serve as an abstraction of the page/note itself. Which then facilitates composing and mixing concepts together to generate new ideas.
• I would expect the title "Conditional Modifier" (a lexical title) (singular not plural on purpose) to be a page about the concept or object itself. I'd expect this noun phrase to be a definition or encyclopedic knowledge on what a conditional modifier is.

[vburzynski]

I'd be interested in finding a standardized layout, form, or pattern (informal definition) for guideline pages. Something akin to Pattern Forms. Though I acknowledge that we aren't generating formal patterns here, but more sharing best practices, maxims, preferences, conventions, heuristics, lessons learned, etc.

Still, I would argue that creating a structural consistency across guideline pages would facilitate the readability, scanability, searchability, navigability, and organization of the pages. This would encourage internal consistency across a diverse set of guidelines.

I'm not sure what this form would look like (we might need to build out several pages first and discover the common pattern). Perhaps something like: problem, solution, tradeoffs (indications/contraindications; pros/cons; advantages/disadvantages).

I could also see us using a variant the portland form as well...

• opening statement — roughly cover the description, problem, context, and forces to be resolved
• therefore... — thesis, solution, the action to take, resolution, resulting context
• but... — antithesis, contraindications, exceptions, things that contrast the concepts already mentioned

It enacts just enough structure to assist with pages being consistent while still being ad hoc and flexible.

[rdnewmanbot]

If we allow inline conditions in the middle of the body, how do we feel about the style that attempts to "flatten" a conditional to fewer lines like:

def can_edit_admin_post?
  return owner? unless admin?
  true
end

In practice, that ending true is a (slight) smell to me. I'd avoid it. If I have to explicitly write out true or false as the body in a conditional (implicit or otherwise), I tend to look for another way. The one you list below is more likely to me.

I do occasionally write an interrogative function through with true or false as the return value in a guard statement without hesitation, such as

def can_proceed?
  return false unless admin?

  active? && configured?
end

That of course could readily be instead written as admin? && active? && configured?. Though this is a contrived example (I know I've had better cases), I see one advantage in this one: the guard statement separates permission from non-permission states. IOW, the nature of admin? is pulled away from the core intention of the function (as artificial as it is). So I'm using the guard statement to help communicate what is important and what is more housekeeping to the reader.

That wasn't the point of your question. We're not saying please use guard statements where you can, but that they can be helpful and, given the scope of this particular line in the guide ("Limit use..."), they are an appropriate place to use a conditional modifier than the full form.

Also, I was trained earlier by default Rubocop to always put a blank line after a single guard statement (or after the last guard statement in a series of them), so I would have wanted a blank line after return owner? unless admin? to help set it apart. (I still like the bare true, but different issue.)