-
Notifications
You must be signed in to change notification settings - Fork 2
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
Refactor by creating Term, Amount and ExciseTaxRate classes #13
Open
mi-wada
wants to merge
1
commit into
moneyforward:main
Choose a base branch
from
mi-wada:refactor
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
module Jct | ||
class Amount | ||
attr_reader :value | ||
|
||
def initialize(amount) | ||
# You can convert Integer/BigDecimal/Float/String/Rational classes to Rational, | ||
# but the `amount` does not accept BigDeciaml, Float and String for the following reasons. | ||
# - Rational objects may be implicitly converted to BigDecimal type when performing arithmetic operations using BigDecimal and Rational. | ||
# - Also, when you try to convert BigDecimal to Rational, the resulting value may not be Rational, but BigDecimal. | ||
# - Float is not accepted because it is not suitable for calculating sales tax rates. | ||
# - String is not accepted because an exception is raised by data that cannot be converted, such as 1.1.1, for example. | ||
raise ArgumentError.new('amount data-type must be Integer or Rational') unless amount.is_a?(Integer) || amount.is_a?(Rational) | ||
|
||
@value = amount | ||
end | ||
|
||
def per_day(number_of_days) | ||
raise ArgumentError.new('number_of_days data-type must be Integer') unless number_of_days.is_a?(Integer) | ||
raise ArgumentError.new('number_of_days must be greater than zero') if number_of_days <= 0 | ||
|
||
Rational(value, number_of_days) | ||
end | ||
end | ||
end |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
module Jct | ||
class ExciseTaxRate | ||
attr_reader :rate, :term | ||
|
||
def initialize(rate:, term:) | ||
@rate = rate | ||
@term = term | ||
end | ||
|
||
def is_in_effect_on?(date) | ||
term.includes?(date) | ||
end | ||
|
||
def is_in_effect_for?(term) | ||
self.term.overlaps_with?(term) | ||
end | ||
end | ||
end |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
module Jct | ||
class Term | ||
attr_reader :start_on, :end_on | ||
|
||
def initialize(start_on:, end_on:) | ||
raise ArgumentError.new('start_on data-type must be Date') unless start_on.is_a?(Date) | ||
raise ArgumentError.new('end_on data-type must be Date') unless end_on.is_a?(Date) | ||
raise ArgumentError.new('start_on must not be after than end_on') if start_on > end_on | ||
|
||
@start_on = start_on | ||
@end_on = end_on | ||
end | ||
|
||
def includes?(date) | ||
start_on <= date && date <= end_on | ||
end | ||
|
||
def overlaps_with?(term) | ||
# Patterns that self and term **donot** overlap | ||
# term.end_on < self.start_on | ||
# self: |---| | ||
# term: |---| | ||
# OR | ||
# self.end_on < term.start_on | ||
# self: |---| | ||
# term: |---| | ||
!(term.end_on < start_on || end_on < term.start_on) | ||
end | ||
|
||
def number_of_days | ||
(end_on - start_on).to_i + 1 | ||
end | ||
|
||
def number_of_days_that_overlap_with(term) | ||
return 0 unless overlaps_with?(term) | ||
|
||
if start_on <= term.start_on && term.end_on <= end_on | ||
# self: |-------| | ||
# term: |---| | ||
term.number_of_days | ||
elsif term.start_on <= start_on && end_on <= term.end_on | ||
# self: |---| | ||
# term: |-------| | ||
number_of_days | ||
elsif term.start_on <= start_on && term.end_on <= end_on | ||
# self: |---| | ||
# term: |---| | ||
Term.new(start_on: start_on, end_on: term.end_on).number_of_days | ||
else | ||
# self: |---| | ||
# term: |---| | ||
Term.new(start_on: term.start_on, end_on: end_on).number_of_days | ||
end | ||
end | ||
end | ||
end |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
require_relative '../test_helper' | ||
|
||
Amount = Jct.const_get(:Amount) | ||
|
||
class AmountTest < Minitest::Test | ||
def test_initialize_failure | ||
error = assert_raises ArgumentError do | ||
Amount.new('100') | ||
end | ||
assert_equal 'amount data-type must be Integer or Rational', error.message | ||
end | ||
|
||
def test_initialize | ||
[ | ||
100, | ||
Rational(100), | ||
0, | ||
Rational(0), | ||
-100, | ||
Rational(-100) | ||
].each do |amount| | ||
assert_equal amount, Amount.new(amount).value | ||
end | ||
end | ||
|
||
def test_per_day_failure | ||
amount = Amount.new(100) | ||
|
||
error = assert_raises ArgumentError do | ||
amount.per_day(1.1) | ||
end | ||
assert_equal 'number_of_days data-type must be Integer', error.message | ||
|
||
error = assert_raises ArgumentError do | ||
amount.per_day(0) | ||
end | ||
assert_equal 'number_of_days must be greater than zero', error.message | ||
end | ||
|
||
def test_per_day | ||
amount = Amount.new(100) | ||
|
||
assert_equal Rational(amount.value, 10), amount.per_day(10) | ||
end | ||
end |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Until now, the number of days has been calculated using
(start_on..end_on).count
. However, from the following simple benchmark, this method was faster than(start_on..end_on).count
, so I changed it.