Patch to partially add recursion to budgets

[Chris Shoemaker]

On Mon, Oct 02, 2006 at 08:26:07PM -0500, Gregory Alexander wrote:
> Alright, I have a patch.
> 
> This may start a flame, but I added a new GNC_ERROR_UNSET error code.
> I added this because none of the existing errors was really
> appropriate for a value that simply wasn't set yet.  I'm open to other
> names like GNC_ERROR_UNINITIALIZED, and would reluctantly change it to
> GNC_ERROR_ARG if there are strong objections to adding a new error
> code.  I already caught one place where the number of errors was
> hard-coded, so I replaced that with a new GNC_ERROR_MIN value.

If the error value was the only way to convey "unset" I could see
adding a code for it, but I'm thinking it's better to handle it
at the budget-level.  E.g.

gboolean
gnc_budget_is_account_period_value_set(GncBudget *budget, Account *account,
                                       guint period_num)
{
    const GUID *guid;
    KvpFrame *frame;
    gchar path[BUF_SIZE];
    gchar *bufend;

    frame = qof_instance_get_slots(QOF_INSTANCE(budget));
    guid = xaccAccountGetGUID(account);
    bufend = guid_to_string_buff(guid, path);
    g_sprintf(bufend, "/%d", period_num);
    return (kvp_frame_get_value(frame, path) != NULL)
}


and then just "unset" the value if an error code is passed to 
gnc_budget_set_account_period_value():

@@ -234,7 +251,10 @@
     bufend = guid_to_string_buff(guid, path);
     g_sprintf(bufend, "/%d", period_num);

-    kvp_frame_set_numeric(frame, path, val);
+    if (gnc_numeric_check(val))
+        kvp_frame_set_value(frame, path, NULL);
+    else
+        kvp_frame_set_numeric(frame, path, val);
     qof_instance_set_dirty(&budget->inst);
     gnc_budget_commit_edit(budget);

In that case, adding an error code isn't really needed.

> 
> User interface changes:
> 
> 1.) Budget values of 0/1 are now displayed when editing as $0.00 (as
> appropriate) instead of being left blank.

okay, good.  Now, zero is a legit budget value. 

> 2.) If the user removes all text when editing a budget value entry, it
> will be initialized to GNC_ERROR_UNSET.

With code above, any error code will do, and will actually _unset_ the value.

> 3.) GNC_ERROR_UNSET is displayed when editing as a blank budget value.

?? It should be just blank, right?

> 4.) cleanup/debug: Other errors display as "error" when editing.

That's a good idea.

> 5.) In the existing budget report, entries with a value of
> GNC_ERROR_UNSET display as "." instead of a currency value.  Other
> errors continue to display as a single "$" (as appropriate.)

I'd prefer that the report display and editor display were more
similar, like just "" and "error", but I might change my mind if I
actually saw it.

> 
> What I haven't been able to figure out is how to have new budget
> entries default to GNC_ERROR_UNSET instead of 0.  I think changing the
> default to GNC_ERROR_UNSET would be the correct behavior, although
> again, this is open to discussion.

Well, I think the default should be "unset" as per above, and I think
it's just automatic.  Nothing's there until it's set.

> 
> Comments, concerns?
> 
> Thanks!

What do you think?

> 
> 
> BTW, I'm also working on a fancier budget report, but it isn't
> currently in a state appropriate for public consumption (read: I
> naively reimplemented gnc:numeric-add, among other ugly things.)  I'll
> submit that in a separate patch once I get it cleaned up.
> 

Looking forward to it.

> 
> 
> Replies below here, but mostly just agreeing with what you said.
> 
> On 9/28/06, Chris Shoemaker  <c.shoemaker at cox.net> wrote:
> >On Thu, Sep 28, 2006 at 02:30:16AM -0500, Gregory Alexander wrote:
> >> Now, let's assume the user doesn't actually care about the Non-Bills
> >> category.  It's just a placeholder (I can come up with some better 
> >reasons
> >> for this, but I want to keep using the same account structure.)
> >>
> >> ...
> >> The user would probably prefer to have one of the following happen.
> >>
> >> Option 1 (what I suggested earlier): fill in from the bottom-up.  This 
> >gives
> >
> >I'm not so sure this is generally good behavior.  Imagine that the
> >user only cares to budget for _some_ of the children accounts, and not
> >the parent.  In that case, using the sum of the budgeted children as
> >the parent's budgeted value isn't correct.
> 
> I'm not convinced by your budget for _some_ argument, since they will
> be counted in the actual total for that category.  However, since the
> overall budget for the parent may be different from the sum of the
> children, I agree that automatically summing isn't a very good general
> practice.  

Yes, that's all I meant.

-chris

> Since there isn't a clean or easy way to add this as an
> option to the interface, I agree that the correct behavior is to not
> implement summing.
> 
> >> Option 2: Mark entry as unbudgeted.
> >>
> >
> >Hmm.  I expected that the normal usage of the budget report would be
> >to only include the accounts that you budgeted for.  But, I can
> >imagine some cases where it might be interesting to see the accounts
> >you budgeted for alongside the accounts you didn't, in the same
> >report.
> >
> >That sounds useful.
> >
> >> Does this seem like a more reasonable path?
> >
> >Yes, it does.  And easier, too.  It could involve:
> >
> >1) return one of the of the gnc_numeric error states from the
> >get_budgeted_value() when it was unspecified.
> >
> >2) Allow set_budgeted_value() to "unset" the value if a bad numeric is
> >supplied.  (or, provide an unset_budgeted_value()).
> >
> >3) Teach the report to do something pretty for unspecified budget
> >values.
> >
> >Are you interested in patching that up?
> 
> See above.