[RFC] Fix #42: Work with suntrust double column csv files #48
Conversation
BlackEdder
changed the title
|
The commit history on this is a bit of a mess, because I tried multiple approaches. Therefore some of the later commits undo much of the earlier commits. I can rebase and clean that up if requested. |
| Reckon::MoneyColumn.new( ["", "AB-2.00C"] ) ).should == [ | ||
| Reckon::Money.new( 1.00 ), Reckon::Money.new( -2.00 ) ] | ||
| Reckon::MoneyColumn.new( ["AB1.00C", "AB0C"] ).merge!( | ||
| Reckon::MoneyColumn.new( ["AB0C", "AB-2.00C"] ) ).should == [ |
There was a problem hiding this comment.
Do you have an example CSV that has numbers in this format? It might be worth adding above... or is this the Suntrust one? I just don't see why that one was having issues.
There was a problem hiding this comment.
I have no real example, but thought it was nice to allow arbitrary postfix/prefix with numbers in them. In many ways this is more for the columns that are likely to not be money, but something like account numbers. It will still parse them as money and give them the benefit of the doubt.
I've updated the description of this PR to explain why the suntrust csv didn't work.
|
I don't have a strong opinion about rebasing the history. Up to you if you want to. |
|
Added explanations and named regexes to the from_s method. Hope that is a lot clearer now. If don't really have a strong opinion about rebasing, so let's keep it like this. |
|
Sorry for the delay here. What's the status on this? |
|
I'm happy with this. In general this change results in a more robust system and adds support for an additional csv format. |
|
LGTM! Go ahead and merge :) |
[RFC] Fix #42: Work with suntrust double column csv files
The problem:
Suntrust csv has two money columns, one for debit and one for credit. Reckon used to deal with this by appending two columns (space separated) with each other and then testing them for a money column. This works fine if one column is always empty -> ["2.00", ""], ["", "1.00"] merged is ["2.00" + " " + "", "" + " " + "1.00"]
but the suntrust one had 0.00 instead of "" -> ["2.00", "0"], ["0", "1.00"] merged is ["2.00" + " " + "0", "0" + " " + "1.00"], which clearly leads to problems.
The solution
First convert the columns to money columns and then merge them like that. The only downside of this solution is that when converting to money we lose some information (prefixes and suffixes). This initially led to some problems when converting two columns that had numbers in them, but clearly where not money columns (account numbers etc). I solved this by making the Money.from_s function stricter.