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

Cop idea: prefer .maximum and .minimum over .pluck & .max or .min #1205

Open
ydakuka opened this issue Dec 6, 2023 · 1 comment · May be fixed by #1371
Open

Cop idea: prefer .maximum and .minimum over .pluck & .max or .min #1205

ydakuka opened this issue Dec 6, 2023 · 1 comment · May be fixed by #1371

Comments

@ydakuka
Copy link

ydakuka commented Dec 6, 2023

Describe the solution you'd like

https://www.fastruby.io/blog/rails/performance/writing-fast-rails-part-2 (rails 6.0)

https://blog.saeloun.com/2021/03/17/rails-enumerable-maximum-and-minimum/ (rails 7.0)

# bad
Loan.pluck(:amount).min
Loan.pluck(:amount).max

# good
Loan.minimum(:amount)
Loan.maximum(:amount)
@ydakuka ydakuka changed the title Cop idea: prefer ActiveRecord::Calculations some methods over Enumerable some methods Cop idea: prefer .maximum and .minimum over .pluck & .max or .min Dec 14, 2023
@vinita000
Copy link

vinita000 commented Dec 28, 2023

@ydakuka
I've reviewed the suggested cop idea, and I think it's a valuable addition. Utilizing ActiveRecord::Calculations methods like maximum and minimum not only enhances code readability but also brings potential performance improvements.

The proposed solution, which suggests using ActiveRecord::Calculations methods (maximum and minimum) instead of pluck followed by min or max, is a good idea. Here are a few reasons why this solution might be beneficial:

Performance Improvement:

ActiveRecord::Calculations methods are optimized for database-level calculations, which can result in more efficient queries.
Using Loan.minimum(:amount) or Loan.maximum(:amount) often translates to a single SQL query, while Loan.pluck(:amount).min or Loan.pluck(:amount).max involves fetching all records and performing the calculation in Ruby.

Readability:

The intent of the code becomes clearer when using minimum and maximum directly, making it easier for other developers (or even yourself) to understand the purpose of the code.

Code Conciseness:

The alternative code (Loan.pluck(:amount).min or Loan.pluck(:amount).max) is longer and involves multiple method calls. Using minimum and maximum directly makes the code more concise.

Consistency:

Favoring ActiveRecord::Calculations methods promotes consistency in your codebase. Developers working on the project will use a standardized approach for such queries.

Compatibility:

As Rails evolves, methods like pluck followed by min or max might become less favorable or less efficient. Using the recommended ActiveRecord::Calculations methods ensures compatibility with future Rails versions.

I support the idea of encouraging developers to adopt this pattern, and I believe it aligns well with the practices mentioned in the provided references.

Looking forward to hearing more thoughts from the community on this proposed cop!

🚀 Thanks!

@fatkodima fatkodima linked a pull request Sep 22, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants