Hi Xinruo,
thank you very much!
I've done a first read-through. I like that you've added functionality
and reduced the code.
It's nice to separate concerns where possible for easier code
comprehension. Is it necessary to do
- checking that a transaction's postings balance, and filling in
missing amounts
- checking historical balance assertions, and resetting account
balances
all at once ? Can these be separated at all ?
Do you know if there's any performance impact ? The "quickbench" and
"samplejournals" make targets may be useful.
A more serious concern is that I find this ledger feature as currently
designed pretty confusing to use and to understand the semantics of.
I've discussed this on #ledger in the past, but don't yet remember all
the details (and there's no channel log). It feels like too many
interlocking meanings of the = symbol and missing amounts. Maybe
balance assertion and assignment should have different symbols ?
I'll try it here and we can discuss more. I'd welcome any other
opinions.
-Simon
On Sat, Jul 27, 2013, at 08:15 AM, Xinruo Sun wrote:
To support balance resetting, we need to keep track of "current"
account balances. The algorithm for balance
assertion/assignment/resetting is basically the same: derive the
amount from the assertion, and compare it to the actual amount, if
any.
If there are multiple assertion postings for the same account in one
transaction, it's not always possible to derive&update posting by
posting. So when this happens, I don't update the balances mapping
until all postings are checked for a transaction. This seems to be
the semantics of ledger 3.0 also. So the tests are also
updated/modified to reflect these situation.
_______________________________________________________________
You can merge this Pull Request by running
git pull https://github.com/xiaoruoruo/hledger assignment
Or view, comment on, or merge it at:
[1]https://github.com/simonmichael/hledger/pull/129
Commit Summary
* support balance resetting and make assertion semantics same with
ledger
File Changes
* M [2]hledger-lib/Hledger/Data/Journal.hs (75)
* M [3]hledger-lib/Hledger/Data/Transaction.hs (39)
* M [4]hledger-lib/Hledger/Utils.hs (8)
* M [5]tests/balance-assertions.test (70)
Patch Links:
* [6]https://github.com/simonmichael/hledger/pull/129.patch
* [7]https://github.com/simonmichael/hledger/pull/129.diff
References
1. https://github.com/simonmichael/hledger/pull/129
2. https://github.com/simonmichael/hledger/pull/129/files#diff-0
3. https://github.com/simonmichael/hledger/pull/129/files#diff-1
4. https://github.com/simonmichael/hledger/pull/129/files#diff-2
5. https://github.com/simonmichael/hledger/pull/129/files#diff-3
6. https://github.com/simonmichael/hledger/pull/129.patch
7. https://github.com/simonmichael/hledger/pull/129.diff
--
---
You received this message because you are subscribed to the Google Groups
"Ledger" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
