Skip to content

Conversation

@samarthnaikk
Copy link

Overview: What does this pull request change?

This pull request refactors the Torus class in manim/mobject/three_d/three_dimensions.py by renaming the field self.r to self.minor_radius. This change improves code clarity and reduces potential confusion with the self.R (major_radius) field. All internal usages of the field have been updated accordingly.

  • Refactored Torus class by renaming the ambiguous self.r attribute to self.minor_radius.

Motivation and Explanation: Why and how do your changes improve the library?

The motivation for this change is to enhance code quality and maintainability.

  • Clarity: The field self.r was used to represent the minor radius of the torus, while self.R represents the major radius. The minimal visual difference between r and R can easily lead to misinterpretation, bugs, and confusion, especially for new contributors.
  • Improved Maintainability: Renaming self.r to the more descriptive self.minor_radius makes the purpose of the field immediately clear, which simplifies future maintenance and development.
  • Tool Conformance: This change resolves a "code smell" flagged by static analysis tools like SonarQube regarding unclear variable names.

The change was implemented by renaming the attribute and using refactoring to update all its occurrences within the Torus class, ensuring functionality remains identical.

Links to added or changed documentation pages

This change primarily affects the internal implementation of the Torus class. The docstrings within manim/mobject/three_d/three_dimensions.py have been reviewed to ensure they reflect the new attribute name. No public-facing documentation pages were directly affected.

Further Information and Comments

The field self.r in the Torus class has been renamed to self.minor_radius, and all usages within the class have been updated. The implementation was verified against related test files, and no regressions were found. The code is now more maintainable and aligns with best practices for clear naming conventions.

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

this PR should solve #4271

@samarthnaikk
Copy link
Author

Are there any changes that are still expected ? @henrikmidtiby

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

Successfully merging this pull request may close these issues.

1 participant