Skip to content

Ex-Date Support: CSV Import/Export#5615

Open
ZfT2 wants to merge 3 commits intoportfolio-performance:masterfrom
ZfT2:csv_ex_date
Open

Ex-Date Support: CSV Import/Export#5615
ZfT2 wants to merge 3 commits intoportfolio-performance:masterfrom
ZfT2:csv_ex_date

Conversation

@ZfT2
Copy link
Copy Markdown
Contributor

@ZfT2 ZfT2 commented Apr 9, 2026

added support for ex-date field for the CSV Import/Export.

@gamue
Copy link
Copy Markdown
Contributor

gamue commented Apr 9, 2026

Let's link #5574 here :)

- try to fix test run
Copy link
Copy Markdown
Contributor

@gamue gamue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me :) Thanks!

slf4j.api,
net.minidev.accessors-smart,
net.minidev.json-smart,
org.apache.commons.commons-io,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the background of this? Wondering as the new code isn't using them

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
AccountTransaction transaction = new AccountTransaction();
var transaction = new AccountTransaction();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I like the explicit typing more :) But I agree in this case var would also be fine :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I changed it to var, the secound finding also. :)

@Test
public void testLeavesExDateEmptyForPortfolioTransactions() throws Exception
{
PortfolioTransaction transaction = new PortfolioTransaction();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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

Copy link
Copy Markdown
Contributor Author

@ZfT2 ZfT2 Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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$
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(personal opinion) for something like this I like stream-api with ofNullable(..).map(..).orElse()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants