Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New linter to suggest moving complex conditional expressions to boolean function/variable? #1830

Open
IndrajeetPatil opened this issue Dec 10, 2022 · 3 comments · May be fixed by #2676
Open
Assignees

Comments

@IndrajeetPatil
Copy link
Collaborator

Preamble

Complex conditional expressions can make the code difficult to read because they interrupt the "flow" of the main program.

If one instead extracts them to a boolean function/variable, the new conditional expression much more readable and, as a side effect, the new function/variable introduces a reusable abstraction in the codebase.

Example

Actual

if (looks_like_a_duck(x) && 
    swims_like_a_duck(x) && 
    quacks_like_a_duck(x)) {
  ...
}

Suggested-1: boolean function

is_duck <- function(x) {
  looks_like_a_duck(x) &&
    swims_like_a_duck(x) &&
    quacks_like_a_duck(x)
}

if (is_duck(x)) {
  ...
}

Suggested-1: boolean variable

is_duck <- 
  looks_like_a_duck(x) &&
  swims_like_a_duck(x) &&
  quacks_like_a_duck(x)

if (is_duck) {
  ...
}

Note that this is the simplest cases, but one can imagine that the expression depend on different objects, and so the boolean function can be multi-parameter (e.g. is_duck(x, y, z)).

Configurable

As for what constitutes a complex conditional expression can be configurable. The default will be 2L. That is, any expression with more than two operands will set off this linter. But users can set this to a higher or a lower value (e.g. if someone prefers to always use a boolean function in a conditional expression to improve readability).

@AshesITR
Copy link
Collaborator

Counter-argument to factoring out a method: It moves the behavior somewhere else, so if you want to understand the code, you need to read in two locations and remember the definition.

This doesn't apply to the "variable" case.

@IndrajeetPatil
Copy link
Collaborator Author

Counter-argument to factoring out a method: It moves the behavior somewhere else, so if you want to understand the code, you need to read in two locations and remember the definition.

But won't this criticism apply in general to any refactoring where part of the code is moved to a new routine/method/function? In other words, this counter-argument speaks against the whole idea of modularizing the codebase using smaller, reusable functions with expressive names.

@IndrajeetPatil
Copy link
Collaborator Author

Hadley also seems to think along similar lines: tidyverse/style#197 (comment)

@IndrajeetPatil IndrajeetPatil self-assigned this Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants