feat: add PercentageFloat method for decimal percentage support#167
Open
walle250ai wants to merge 2 commits into
Open
feat: add PercentageFloat method for decimal percentage support#167walle250ai wants to merge 2 commits into
walle250ai wants to merge 2 commits into
Conversation
Adds Money.Percentage(percentage int64) (*Money, error) that returns a new Money representing the given integer percentage of the original amount. Uses the existing calculator.allocate path (amount*p/100) for consistency with Allocate/Split. Returns an error for percentages outside [0, 100]. Tests cover normal cases, boundary values, and invalid inputs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Percentage(int64) only handles whole-number rates; real-world tax and discount rates like 12.5% or 6.25% require decimal support. PercentageFloat(float64) converts the rate to a scaled numerator/denominator (precision 1e9) and delegates to the existing calculator.allocate path so all arithmetic stays in integer space. Returns an error for rates outside [0, 100]. Tests cover 12.5% of $200→$25, 6.25% of $800→$50, 33.3% of $300→$99 (documented truncation), boundary values, and invalid inputs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewer's GuideAdds percentage-based calculation helpers to Money, including a new float-based API that keeps arithmetic in integer space, with comprehensive validation and tests for both integer and float percentages. Sequence diagram for PercentageFloat delegation to calculatorsequenceDiagram
actor Caller
participant Money
participant mutate
participant calc
Caller ->> Money: PercentageFloat(percentage float64)
Money ->> Money: validate percentage in [0.0, 100.0]
Money ->> Money: compute precision, numerator, denominator
Money ->> mutate: use calc to allocate amount
mutate ->> calc: allocate(m.amount, numerator, denominator)
calc -->> mutate: allocatedAmount int64
mutate -->> Money: allocatedAmount int64
Money ->> Money: construct new Money(allocatedAmount, m.currency)
Money -->> Caller: *Money, nil
Class diagram for Money with new percentage helpersclassDiagram
class Money {
int64 amount
Currency currency
Allocate(rs ...int) []*Money, error
Percentage(percentage int64) *Money, error
PercentageFloat(percentage float64) *Money, error
Display() string
}
class Mutate {
Calculator calc
}
class Calculator {
allocate(amount int64, numerator int64, denominator int64) int64
}
Money --> Mutate : uses mutate
Mutate --> Calculator : uses calc
%% New methods introduced in this PR
Money : Percentage(percentage int64) *Money, error
Money : PercentageFloat(percentage float64) *Money, error
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
PercentageFloat, consider rounding the scaled percentage (e.g.,int64(math.Round(percentage * precision))) instead of truncating the float64 multiplication result to reduce the impact of floating-point representation error on boundary cases. - The range checks and error messages for
PercentageandPercentageFloatare duplicated; extracting a shared validation/helper or a shared error value would keep behavior consistent and simplify future changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `PercentageFloat`, consider rounding the scaled percentage (e.g., `int64(math.Round(percentage * precision))`) instead of truncating the float64 multiplication result to reduce the impact of floating-point representation error on boundary cases.
- The range checks and error messages for `Percentage` and `PercentageFloat` are duplicated; extracting a shared validation/helper or a shared error value would keep behavior consistent and simplify future changes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Percentage(int64)only supports whole-number rates. Real-world tax and discount calculations frequently require decimal percentages (12.5% VAT, 6.25% sales tax, etc.) that cannot be represented.Solution
Adds
PercentageFloat(percentage float64) (*Money, error)that accepts afloat64rate in[0.0, 100.0]while keeping all arithmetic in integer space.Implementation approach
Instead of multiplying
amountby a float, the method scales the percentage to a large integer numerator/denominator pair (precision = 1e9) and delegates to the existingcalculator.allocate(amount, numerator, denominator)path:This avoids floating-point amounts entirely, consistent with the library's design.
Tests
PercentageFloat(12.5)on 200PercentageFloat(6.25)on 800PercentageFloat(33.3)on 300PercentageFloat(0.0)PercentageFloat(100.0)All existing tests continue to pass.
🤖 Generated with Claude Code
Summary by Sourcery
Add integer- and float-based percentage calculation helpers to Money with validation and coverage tests.
New Features:
Tests: