Skip to content

Refactor: Rename field r to avoid confusion with field R #4271

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

Open
murilodepa opened this issue Jun 7, 2025 · 0 comments
Open

Refactor: Rename field r to avoid confusion with field R #4271

murilodepa opened this issue Jun 7, 2025 · 0 comments

Comments

@murilodepa
Copy link

murilodepa commented Jun 7, 2025

Description:

  • The field self.r, defined in the context of the variable minor_radius, may cause confusion with the field self.R defined on line 1241. This can make the code harder to read and maintain, especially for new contributors or in scenarios where both fields are used in close proximity.
  • Renaming the field r to something more descriptive (e.g., minor_radius) can prevent ambiguities and align the code with best practices for naming.

Motivation:

  • Clarity: The visual difference between r and R is minimal, which might lead to misinterpretation or bugs in future implementations.
  • Improved Maintainability: A more descriptive name enhances the understanding of the field's purpose.
  • Tool Conformance: This change resolves the code smell flagged by the SonarQube static analysis tool.

Impact on Existing Code:

  • Renaming the field will require updating all occurrences of self.r in the code to the new name. This can be safely handled using refactoring tools. Existing tests should be run to ensure the functionality remains intact.

Proposed Solution:

Rename the field self.r to self.minor_radius or another name that consistently reflects its purpose, in this file "manim/mobject/three_d/three_dimensions.py".

For example:

#Before
self.r = minor_radius

#After
self.minor_radius = minor_radius

References:

  • SonarQube Report: Code Smell (Blocker)
  • Affected line: self.r (current repository code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

No branches or pull requests

1 participant