Skip to content

Comments

refactor(math): no need for a dedicated type for Constant#728

Open
Eh2406 wants to merge 1 commit intotxpipe:mainfrom
Eh2406:remove-const
Open

refactor(math): no need for a dedicated type for Constant#728
Eh2406 wants to merge 1 commit intotxpipe:mainfrom
Eh2406:remove-const

Conversation

@Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented Feb 5, 2026

While trying to figure out what struct Constant was being used for I tried some compiler driven development. That is to say, I removed the definition and looked at where the error messages were. When all was said and done, I did not figure out what justified the New Type and the resulting code seemed somewhat easier to read. (I'm happy to close the PR if I've missed something important and the abstraction does pull its weight.)

Summary by CodeRabbit

  • Refactor
    • Simplified internal mathematical constant handling to improve code efficiency and reduce complexity.

@coderabbitai
Copy link

coderabbitai bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

Refactored constants in the math module from custom Constant wrapper structs to direct IBig LazyLock bindings, removing all .value accessor patterns and updating all usages throughout the file accordingly.

Changes

Cohort / File(s) Summary
Constant Refactoring
pallas-math/src/math_dashu.rs
Replaced custom Constant wrapper type with direct LazyLock<IBig> bindings for TEN, PRECISION, EPS, ONE, ZERO, and E. Updated all arithmetic operations, comparisons, and helper functions to use direct IBig references instead of .value accessors. Adjusted dereference patterns throughout to align with the new constant types.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

A rabbit hops through cleaner code,
Unwrapping constants from their load,
No more .value calls to make,
Direct LazyLocks for goodness' sake!
Simpler paths where wrappers once hid,
A tidy refactor, nicely done, well rid! 🐰✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title clearly summarizes the main change: removing the Constant struct wrapper and using direct IBig LazyLock constants instead, which aligns perfectly with the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Eh2406 Eh2406 changed the title No need for a dedicated type for Constant refactor(math): no need for a dedicated type for Constant Feb 9, 2026
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 this pull request may close these issues.

1 participant