Advanced portfolio report [another version]

Morrison J. Chang mjchang at ix.netcom.com
Thu Nov 29 23:00:59 EST 2007


On Thu, 2007-11-29 at 13:34 -0800, Andrew Sackville-West wrote:
> On Wed, Nov 28, 2007 at 11:38:34PM -0500, Morrison J. Chang wrote:
> > 
> > Notes:
> > In this version I created two new lists for holding the sold-value and
> > sold-basis-value. I apply stock splits against the basis in
> > basis-modify.
> 
> I've only quickly skimmed your code. ISTM that the basis manipulation
> should be as contained as possible. You've broken our the handling of
> splits/mergers into YANFunction, when its really not necessary. A
> little work with pencil-paper tells me that you can abstract the
> splits/mergers into one case with a simple two-step algorithm:
> 
> -  Figure out the ratio that results from the change in number of units
> -  Apply that ratio to every cell in the basis list (recursion makes
>    this trivially simple to implement for any type of basis-list)
> 
> It doesn't matter what kind of basis you're taking. I've moved the
> splits/mergers code into the basis-builder function. It's really not
> the job of anything *outside* the basis-builder to care one way or
> another what kind of basis-changing operation is going on. A simple
> check for a zero value tells basis builder whether to handle as a
> buy/sell or a split/merger. 

For a while I was thinking of arguing for a modularization of the basis
math. Frankly if the function is bigger than a page, then I have to be
really mindful of the paren matching (yes I use emacs but still....).
That and the fact that I was playing around with the map routine.

As an aside do you know if map or tail-recursive car/cdr is more
efficient?

In any case I also believe some of my code decisions was based on the
old rounding problem which you pointed out a fix for.
 
> 
> What purpose is the soldvaluecoll?  It looks like you total it up but
> never actually use it. Debug? Same with soldbasiscoll?
> 
I think I was going somewhere with it... see below...

> I've just run this report against one of my test files and it fails on
> a sales txn with capital gains.

Odd, I thought I had it. Can you send me the test file?

> The use of (xaccAccountGetSplitList
> current) (line 547 in your report, I use it too) forces iteration over
> every split that connects to the account, but in a sale with gain or
> loss, each txn has more than one split that touches the account. This
> means that the txn gets checked more than once and it affects the
> basis and other calculations once for *each* split in the txn that
> touches the account. This is wrong, and doubles the impact of those
> txns. My version of the report fixes this by paying attention to which
> splits have already been seen in a simple list. We only examine a
> split if we've not seen it before. This is at line ~415 in mine.
> Look at mine too, the (cond)  at around line 460-480, much simplified
> parsing of the splits as compared to yours (lines ~484-553) and the
> original. There is no need to iterate through the splits multiple
> times if you change the order in which they are parsed. The only time
> you have to actually look at the split is when its *not* the stock
> itself and then the cases break down nicely into just a couple things:
> moneyin, moneyout, brokerage (ACCT-TYPE-EXPENSE) or dividend
> (ACCT-TYPE-INCOME). 

Actually I never understood why the moneyin/moneyout stuff was done in
its own loop (maybe currency issues?). I think that is where I was
headed with soldvaluecoll and soldbasiscoll to just get rid of it since
the basis calc is done in the earlier loop. So in this regard the fact
that I'm able to generate what I think are correct results with my code
puzzles me a bit.

> There is one part of mine that is broken in this regard: a stock
> purchase made directly from income -- like an employer's contribution
> to an IRA that automatically purchases shares. I think yours suffers
> from this as well. There is no way to differentiate between a dividend
> and this other case. Probably should be written up in the docs (I
> haven't read how they currently handle this case).
> 

The docs/tutorial shows the different income accounts, but I don't think
it mentions advanced report.


-Morrison



More information about the gnucash-devel mailing list