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

"Replace Pipeline with a function" isn't quite that #4

Open
novaugust opened this issue Mar 21, 2024 · 5 comments
Open

"Replace Pipeline with a function" isn't quite that #4

novaugust opened this issue Mar 21, 2024 · 5 comments

Comments

@novaugust
Copy link

novaugust commented Mar 21, 2024

Replace pipeline with a function

The given example of "replace a pipeline with a function" is really an example of "use a higher order version of a function to reduce the number of functions called and enumerations performed". The reason I bring this up is that if there were further pipes in the example, the refactor wouldn't remove the pipes - it would simply be changing the function call.

# given
list
|> Enum.filter(&is_foo/1)
|> Enum.count()
|> do_more_work()

# the refactor would be
list
|> Enum.count(&is_foo/1)
|> do_more_work()

adobe's styler tool performs a large number of these optimizing refactors across pipes whenever code is formatted via mix format

(the exact filter |> count implementation is here)

@lucasvegi
Copy link
Owner

Hello @novaugust, thank you for the comment!

I completely understand your point and agree with you.

However, it would be very exhausting and voluminous to exemplify all possible scenarios for each refactoring. This refactoring indeed replaces a pipeline (or part of it) with a function. Whether the replacement is complete or partial depends on the scenario. 😄

Besides Adobe Styler, Credo already can identify various distinct opportunities to apply this refactoring.

@novaugust
Copy link
Author

novaugust commented Mar 21, 2024

Sure, I'm not suggesting you change the cited refactor, but rather the name 🤔 i never think of myself doing a "replace a pipeline with a function call" apart from as a step when i've removed functions from a pipeline and now it's a single pipe. is that what this refactor is about, or is it about using higher order functions to gain efficiency and terseness? the former i do all the time, the latter much more rarely (but when the opportunity arises, sure!)

@lucasvegi
Copy link
Owner

I don't oppose changing the name of the refactoring, feel free to make suggestions.

Overall, I believe that the description of the motivation behind this refactoring reflects well the goals of this transformation:

When utilizing a pipeline composed of built-in higher-order functions to transform data, we may unnecessarily generate large and inefficient code. This refactoring aims to replace this kind of pipeline with a function call that composes it or by invoking another built-in function with equivalent behavior. In both cases, this refactoring will reduce the number of iterations needed to perform transformations on the data, thus improving the code's performance and enhancing its readability.

In summary, this refactoring aims to reduce the number of functions in a pipeline by using built-in functions that perform the same task but with less code volume and greater efficiency. This reduction can either eliminate the entire pipeline (like the example) or reduce it to a smaller pipe (not always a single pipe...), but naturally, it depends on the original code structure to determine how it will look after the transformation.

@novaugust
Copy link
Author

In summary, this refactoring aims to reduce the number of functions in a pipeline by using built-in functions that perform the same task but with less code volume and greater efficiency.

Cheers, so the pipe wasn't the important bit, the higher order step was :) "Reduce number of function calls by using higher order functions"?

@lucasvegi
Copy link
Owner

Cheers, so the pipe wasn't the important bit, the higher order step was :) "Reduce number of function calls by using higher order functions"?

This is a great suggestion! At the moment, I can't make this type of update to the repository because it would break some links in the survey, but I will consider this and other suggestions when I am writing the paper with the research results. Thank you very much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants