Introduction of percentage-based price quotation#5454
Introduction of percentage-based price quotation#5454hporten wants to merge 22 commits intoportfolio-performance:masterfrom
Conversation
The deepCopy() addition is covered through testThatDeepCopyIncludesAllProperties().
Will be needed likely in several places. Still to be translated.
Legacy Security objects will remain untouched.
In two cases, it is impossible to tell just from the dividend/coupon document that the instrument is quoted in percentage.
We want to remain backward compatible to snapshots that contain instruments (and transactions) that worked around the lack of support by dividing the number of "shares" by 100.
When the dividend/coupon document contains a nominal value stated in a currency (rather than ST(ück?)) WKN A0GLU5 Nominal USD 5.000,00 or 1.000,000000 EUR A3511H XS2722190795 then we can imply a security with a nominal value that is percentage-quoted. Already assumed elsewhere in the parser.
Tests are passing as expected now. BUG: Old security registrations should not be touched.
The function will respect legacy Security objects that lacked the concept.
I.e. without the "divide by 100" workaround.
Legacy securities keep on supported.
A radio button or combobox would be prettier. But that would require developing a new kind of binding.
Invisible by default.
The protoc run for regeneration of PSecurity still needs to be done.
Since my Maven installation does not match what Eclipse needs,
I ran protoc manually like this:
$ cd name.abuchen.portfolio
$ /usr/local/protobuf/bin/protoc \
--proto_path=src/name/abuchen/portfolio/model \
--java_out=protos \
src/name/abuchen/portfolio/model/client.proto
PS: Since I first build protoc v3.22.2 from source, I likely could
have simply upgraded Maven as well.
Mainly targetting the multi-directional update rules and error checking found in the AbstractSecurityModel base class. To increase coverage, the expected PropertyChange events, currency conversions and more error cases should be checked as well.
Buy and Sell operations now respect the need to divide the quote by 100. The code duplication cries for a utility function.
BUG: The validation error message "Input required for 'Shares'" will stick to shares even when selecting a nominally counted security. Same for 'Nominal'.
Can easily be done in the other sub-classes of AbstractTransaction- Dialog in the same way now.
See the reverse calculation found in PortfolioTransaction. getGrossPricePerShare(). Maybe, a Security.quoteFactor() (being either 1 or 100) would simplify the code.
|
Mir sagt die isPercentageQuoted irgendwie nicht so richtig zu... es ist Mehrarbeit bei der Importerpflege und man müsste alle Importer wo der " |
Don't the importer already have a workaround today? This pull request maintains some kind of backwards compatibility:
We could also decide - once bonds are supported - to not support the "divide by 100" workaround anymore. At the moment it is still open how to support bonds. I am not a big fan of a "percentageQuoted" flag. See #5453 |
buchen
added
enhancement
proof_of_concept
needs discussion
I am confident, that the workaround can easily be hidden inside of a single call: getOrCreateBond(v) call. That one would either create a brand new Bond object or apply the old-style workaround of legacy Security items. It will not be mandatory to port all importers at once. But those that will be adapted, will have simpler code than before. PS: Yes, all fine with dropping the explicit percentage-flag and moving to an implicit calculation based on the type. |
|
All I see here is that the attempt to implement this ends up in chaos and failure, just like the implementation of skippedItem. |
3 participants
This draft of patches introduces a Security.percentageQuote boolean with setter and getter methods. The quotation type can be configured in the Security dialog or will be set automatically when a percentage-based security is recognized on imports. This has been tried out for the case of Deutsche Bank documents.
A few places in the UI and the backend calculation have been adapted. There are likely going to be several statistics that still need adjustement.
Here is how the Buy dialog could look like:
For now, backward compatibility will be kept by leaving old securities entirely untouched. More sophisticated approaches that would would enable a gradual migration of legacy data are thinkable.
The individual commits from the experiment have been kept. For a better understanding of how things developed. Cleanups and refactoring will lead to a smaller delta.
Closes: #5453