Ex-Date Support: CSV Import/Export#5615
Ex-Date Support: CSV Import/Export#5615ZfT2 wants to merge 3 commits intoportfolio-performance:masterfrom
Conversation
|
Let's link #5574 here :) |
- try to fix test run
gamue
left a comment
There was a problem hiding this comment.
looks good to me :) Thanks!
| slf4j.api, | ||
| net.minidev.accessors-smart, | ||
| net.minidev.json-smart, | ||
| org.apache.commons.commons-io, |
There was a problem hiding this comment.
what's the background of this? Wondering as the new code isn't using them
There was a problem hiding this comment.
It seems to be needed for the infinitest run. Without, it fails, at least on my build environment.
| @Test | ||
| public void testExportsExDateForAccountTransactions() throws Exception | ||
| { | ||
| AccountTransaction transaction = new AccountTransaction(); |
There was a problem hiding this comment.
| AccountTransaction transaction = new AccountTransaction(); | |
| var transaction = new AccountTransaction(); |
There was a problem hiding this comment.
Personally, I like the explicit typing more :) But I agree in this case var would also be fine :)
There was a problem hiding this comment.
yes, in general I also like explicit typing more, but the code-base of this project prefers var and I think if the type is repeated directly like in this example it's making sense :) But yes, all about personal preferences
There was a problem hiding this comment.
ok, I changed it to var, the secound finding also. :)
| @Test | ||
| public void testLeavesExDateEmptyForPortfolioTransactions() throws Exception | ||
| { | ||
| PortfolioTransaction transaction = new PortfolioTransaction(); |
There was a problem hiding this comment.
| PortfolioTransaction transaction = new PortfolioTransaction(); | |
| var transaction = new PortfolioTransaction(); |
|
|
||
| var errors = new ArrayList<Exception>(); | ||
| var results = extractor.extract(1, | ||
| Arrays.<String[]>asList(new String[] { "2026-03-31", "2026-03-28", "DE0007164600", "SAP.DE", |
There was a problem hiding this comment.
| Arrays.<String[]>asList(new String[] { "2026-03-31", "2026-03-28", "DE0007164600", "SAP.DE", | |
| List.of(new String[] { "2026-03-31", "2026-03-28", "DE0007164600", "SAP.DE", |
haven't checked out the code, but would this also work? I think the currentl way is quite hard to read
There was a problem hiding this comment.
Yours results in List< String > return type, not List< String[] > ...
| private void writeAccountTransaction(CSVPrinter printer, AccountTransaction accountTransaction) throws IOException | ||
| { | ||
| printer.print(accountTransaction.getDateTime().toString()); | ||
| printer.print(accountTransaction.getExDate() != null ? accountTransaction.getExDate().toString() : ""); //$NON-NLS-1$ |
There was a problem hiding this comment.
(personal opinion) for something like this I like stream-api with ofNullable(..).map(..).orElse()
- PR feedback.
added support for ex-date field for the CSV Import/Export.