Skip to content

SONAPPY-2876: Create rule S7504: Using \"list()\" on already iterable objects is unnecessary and inefficient #5035

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented May 9, 2025

You can preview this rule here (updated a few minutes after each push).

Review

A dedicated reviewer checked the rule description successfully for:

  • logical errors and incorrect information
  • information gaps and missing content
  • text style and tone
  • PR summary and labels follow the guidelines

@github-actions github-actions bot added the python label May 9, 2025
@Seppli11 Seppli11 force-pushed the rule/add-RSPEC-S7504 branch 3 times, most recently from f6d4858 to 4a25880 Compare May 9, 2025 13:10
@Seppli11 Seppli11 force-pushed the rule/add-RSPEC-S7504 branch from 4a25880 to f8fde33 Compare May 9, 2025 13:21
@Seppli11 Seppli11 changed the title Create rule S7504 Create rule S7504: Using \"list()\" on already iterable objects is unnecessary and inefficient May 9, 2025
@Seppli11 Seppli11 changed the title Create rule S7504: Using \"list()\" on already iterable objects is unnecessary and inefficient SONAPPY-2876: Create rule S7504: Using \"list()\" on already iterable objects is unnecessary and inefficient May 9, 2025
@Seppli11 Seppli11 requested a review from joke1196 May 9, 2025 13:34
Copy link
Contributor

@joke1196 joke1196 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just a small comments to make the rule a bit clearer.

@@ -0,0 +1,23 @@
{
"title": "Using \"list()\" on already iterable objects is unnecessary and inefficient",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here for the shape of the title it should be something like

Suggested change
"title": "Using \"list()\" on already iterable objects is unnecessary and inefficient",
"title": "Using \"list()\" on already iterable objects should be avoided",


== Why is this an issue?

Using `list()` on already iterable objects adds meaningless code clutter that doesn't provide any functional value when wrapping an already iterable object. Additionally, it creates unnecessary overhead by generating an intermediate list in memory, which inefficiently consumes memory resources and can degrade performance, especially with large data structures. Iterating directly over the original object is cleaner and more efficient.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good I would just specify more clearly that we mean in for loops. Maybe starting the sentence with: When using for loops...

@Seppli11 Seppli11 force-pushed the rule/add-RSPEC-S7504 branch from b1ff123 to 58c8f3a Compare May 12, 2025 07:59
@Seppli11 Seppli11 force-pushed the rule/add-RSPEC-S7504 branch from 58c8f3a to 89d55da Compare May 12, 2025 08:51
Copy link

Copy link

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

Successfully merging this pull request may close these issues.

2 participants