-
Notifications
You must be signed in to change notification settings - Fork 12
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
solution for booking date bug #18 #23
base: master
Are you sure you want to change the base?
Conversation
fetching changes from mitch0000001/go-hbci/master
hopefully fixed the linting (code climate) bug now |
testdata string | ||
expectedBookingDate string | ||
today string | ||
balanceStartBookingDateString string |
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.
As we are already using the dates as time.Time
in line 202ff we should change the type from string
to time.Time
. Also, the setup gets easier as we don't have to parse the date strings
@@ -15,6 +15,7 @@ import ( | |||
|
|||
// MT940 represents a S.W.I.F.T. Transaction Report | |||
type MT940 struct { | |||
ReferenceDate time.Time |
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.
when I read it correctly you're using ReferenceDate
as a way for dependency injection. As the value is always hardcoded to time.Now
at element.swift.go:30
. What do you think about not exposing it yet and guarding against zero value within the Unmarshal
method at swift/mt940_unmarshaler.go:12
?
monthDiff := int(math.Abs(float64(t.ValutaDate.Month() - t.BookingDate.Month()))) | ||
if monthDiff > 1 { | ||
diff := t.ValutaDate.Sub(t.BookingDate.Time).Hours() / 24 | ||
if diff > 31 { |
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.
Unfortunately I haven't yet understood what we're calculating here and why we're checking for 31.
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.
how may days can be between a booking and a valuta date? I am sure it us less then 31 days.
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.
regarding this thread https://homebanking-hilfe.de/forum/topic.php?t=8970 which may or may not be representative the number of days can be way higher
No description provided.