MyFinances icon indicating copy to clipboard operation
MyFinances copied to clipboard

Code cleanup 160

Open reenu153 opened this issue 8 months ago • 1 comments

Description

This pull request refactors the code primarily to address the code cleanup tasks outlined in issue #160. The refactoring addresses several code smells and aims to improve the readability, maintainability, and structure of the codebase.

Refactoring Details:

  1. Magic Numbers/Constants:

    • Issue: Magic numbers and hardcoded values were scattered across the codebase, making the code harder to maintain.
    • Refactoring:
      • Created a backend/core/constants.py file to define named constants for recurring numerical values (e.g., MAX_LENGTH_STANDARD, MAX_LENGTH_NAME, MAX_LENGTH_DESCRIPTION, DECIMAL_MAX_DIGITS, DECIMAL_PLACES).
      • Replaced hardcoded max_length and decimal constraints in models with the defined constants.
  2. Inappropriate/Unclear Naming:

    • Issue: Some variable and function names were ambiguous and did not clearly convey their purpose.
    • Refactoring:
      • Renamed functions to be more descriptive and Pythonic:
        • _public_storageget_public_storage
        • _private_storageget_private_storage
        • RandomCodegenerate_verification_code
        • RandomAPICodegenerate_api_key
        • upload_to_user_separate_folderget_file_upload_path
      • These changes enhance clarity by explicitly describing the functionality of each function.
  3. Set blank=False for name fields: Ensures name and related fields (e.g., description, event_name) are required across multiple models to prevent missing data.

Checklist

  • [x] Ran the Black Formatter and djLint-er on any new code (checks will fail without)
  • [ ] Made any changes or additions to the documentation where required
  • [x] Changes generate no new warnings/errors
  • [x] New and existing unit tests pass locally with my changes

What type of PR is this?

  • ♻️ Code Refactor

Added/updated tests?

  • 🙅 no, because they aren't needed

Related PRs, Issues etc

  • Related Issue #160

reenu153 avatar Apr 10 '25 11:04 reenu153

Hey @reenu153 / @LaraMerdol,

Thank you for these changes! There's currently a fairly big queue in PRs so I wont be able to merge this right away, but I do appreciate the effort! I'll let you know when I get round to testing and merging this :)

TreyWW avatar Apr 10 '25 12:04 TreyWW