Skip to content
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

Add AbsolutePerformancePieChart and AbsolutePerformancePane #4295

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Mikerion
Copy link
Contributor

Pie chart shows Absolute Invested capital + Delta to show own money vs. earnings

obrazek

Pie chart shows Absolute Invested capital + Delta to show own money vs. earnings
long[] d2 = perf.calculateAbsoluteDelta();
wrapper.delta += d2.length > 0 ? d2[d2.length - 1] : 0L;

position = p.getDescription();
Copy link
Member

Choose a reason for hiding this comment

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

The loop is overwriting the label multiple times. Is this intended?
It "smells" like a side effect if the loop is iteratively changing a member variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually label is used only when it loops over 1 item (but it is not known beforehand), if there is more items, portfolio name is used as label instead.
But yes it would be better to have loop label variable in wrapper temp object.

Comment on lines +235 to +237
var dataSeries = dataSeriesSet.getAvailableSeries().stream()
.filter(ds -> ds.getType() != Type.ACCOUNT && ds.getUUID().contains(nodeId))
.findAny().orElse(null);
Copy link
Member

Choose a reason for hiding this comment

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

You use the ClientSnapshot and then lookup the data series (meant for the charts).

Can't we use the ClientPerformanceSnapshot? It has the list of positions plus all the components that have been held in-between.

@buchen
Copy link
Member

buchen commented Oct 25, 2024

Hi @Mikerion,

thanks for the contribution!

I am wondering if the pie chart is the right way to visualize this information.

Why?
First, the delta can be a negative amount but that is not possible to visualize in the chart.
Second, because we are looking at the complete interval, the invested amount can also be negative (for example I purchased for 100 EUR, then the stock prices quadruples, I sell have of my holdings for 200 EUR).

If we want to visualize it, then maybe a "waterfall" chart is more appropriate (at least that is what the German Excel calls it)

Bildschirmfoto 2024-10-25 um 09 06 32

The chart library does not support such a chart (at least to my knowledge). But I assume the math is also not too hard (no sinus and cosinus ;-). And we could render it vertically to better fit into the pane.

What do you think?

Apart from this, I just glanced of the code. I am wondering if the ClientPerformanceSnasphot is the easier source for the data.

@mierin12
Copy link
Contributor

Hello Mikerion, buchen, waterfall would be excellent ! With maybe the option to have a detailled configuration:

  • Conf 1 : delta vs invested

  • Conf 2 : with splitted delta : unrealized capital gain, realized, interest, dividend, fee, taxe etc, invested. The data from Performance/Calculation.

@Mikerion
Copy link
Contributor Author

Hi @buchen,

main motivation was to have something similar to pie chart from https://www.calculator.net/investment-calculator.html or I've also seen it in some web app dashboard. Main thing is ratio of own money vs. "interest" so that's why pie chart is used. I'm not sure ratio is nicely visible in waterfall chart.

I'm not familiar with ClientPerformanceSnasphot so I'd need to check it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants