tag:infinitekind.tenderapp.com,2009-01-14:/discussions/moneydance-development/7269-capitalgain-calculation-is-not-efficientInfinite Kind: Discussion 2022-07-24T18:20:23Ztag:infinitekind.tenderapp.com,2009-01-14:Comment/537592022022-04-08T22:31:14Z2022-04-09T07:27:26ZCapitalGain calculation is not efficient<div><p>.</p></div>Stuart Beesley (Mr Toolbox)tag:infinitekind.tenderapp.com,2009-01-14:Comment/537592022022-04-09T01:15:16Z2022-04-09T01:15:16ZCapitalGain calculation is not efficient<div><p>I figured out how to make a local copy of CostCalculation and I modified it to pass in the cached transaction set. My timings for this function are now down from 15 seconds to 9. I can deal with that but Just for fun I will see if I can get it down any more but I'm off for a few days.:)</p></div>Bob Btag:infinitekind.tenderapp.com,2009-01-14:Comment/537592022022-04-13T21:01:43Z2022-04-13T21:02:22ZCapitalGain calculation is not efficient<div><p>I'm digging down into performance of CapitalGain calculations. This is the code (build 4071) from InvestUtil which is called to get a capital gain result. One of the parameters is a list of<br></p>
<pre>
<code>List<CapitalGainBuy> buyTxns</code>
</pre>
. I can't see where that list is ever used in the method. This is a pretty expensive calculation when done repeatedly. Have I missed something and it is somehow actually used? If it's not used, I could just pass in an empty list and get the same result.
<pre>
<code>private static CapitalGainResult getAverageCostCapGain(Account sec, List<CapitalGainBuy> buyTxns, SplitTxn sellTxn) {
if (sellTxn != null && sellTxn.getValue() <= 0L) {
return (new CostCalculation(sec, sellTxn.getDateInt())).getGainInfo(sellTxn);
} else {
System.err.println("Encountered non-sale txn computing average cost: " + sellTxn);
return new CapitalGainResult("sell_invalid");
}
}</code>
</pre></div>Bob Btag:infinitekind.tenderapp.com,2009-01-14:Comment/537592022022-04-13T21:11:24Z2022-04-13T21:11:24ZCapitalGain calculation is not efficient<div><p>Just try it?</p>
<p>Try passing None/Null (or empty list) as that parameter. What happens?</p></div>Stuart Beesley (Mr Toolbox)tag:infinitekind.tenderapp.com,2009-01-14:Comment/537592022022-04-13T22:49:04Z2022-04-13T22:49:04ZCapitalGain calculation is not efficient<div><p>So it looks like buyTxns is not used within .getAverageCostCapGain() - so try passing null.</p>
<p>However, .getCostBasisCapGain() also takes buyTxns to pass onto . getAverageCostCapGain() - so again pass null to that.</p>
<p>As you have 'recreated' these classes you should be able to see how these are called and whether buyTxns is needed higher up the calling chain.</p>
<p>I would just try it.</p></div>Stuart Beesley (Mr Toolbox)tag:infinitekind.tenderapp.com,2009-01-14:Comment/537592022022-04-14T16:06:21Z2022-04-14T16:06:21ZCapitalGain calculation is not efficient<div><p>It continues to work when passing in a NULL using the standard md functions.</p></div>Bob Btag:infinitekind.tenderapp.com,2009-01-14:Comment/537592022022-04-14T16:20:35Z2022-04-14T16:20:35ZCapitalGain calculation is not efficient<div><p>Did it speed up?</p></div>Stuart Beesley (Mr Toolbox)tag:infinitekind.tenderapp.com,2009-01-14:Comment/537592022022-04-24T18:19:12Z2022-04-24T18:28:05ZCapitalGain calculation is not efficient<div><p>Yes by about half. I've decided to use my own capital gain calculations instead of the ones provided in InvestUtil for two reasons:</p>
<p>1) my calculations complete in about 500 milliseconds (for 1500 sales txns) compared to about 7 seconds using MD and passing in the null value.</p>
<p>2) I found two bugs in the MD calculations. One where there are multiple sales transactions in a single day and one when there is a stock split between buying and selling. I've fully documented the second (attached if you are interested). I will report this bug.</p>
<p>NOTE: my code does not support allocating by lots and does not calculate short vs long term gains (not needed in Canada).</p></div>Bob B