[Gnucash-changes] r13864 - gnucash/trunk/src/gnome - Remove some unnecessary BeginEdit/CommitEdit calls.

[Chris Shoemaker]

On Wed, May 03, 2006 at 11:32:53AM -0400, Derek Atkins wrote:
> Chris Shoemaker <chris at cvs.gnucash.org> writes:
> 
> > Modified: gnucash/trunk/src/gnome/gnc-plugin-page-register.c
> [snip]
> >    /* Now update the original with a pointer to the new one */
> > -  xaccTransBeginEdit(trans);
> >    kvp_val = kvp_value_new_guid (xaccTransGetGUID(new_trans));
> >    kvp_frame_set_slot_nc(txn_frame, "reversed-by", kvp_val);
> > -  xaccTransCommitEdit(trans);
> >    qof_event_resume();
> 
> I think you may still need this BeginEdit/CommitEdit pair here
> because there's no transactional auto-commit in setting the
> kvp.

Indeed.  Good catch!  Now that I look more closely, the original code
wasn't correct either.  The transaction being reversed is never
dirtied, even though it's modified.  I'll clean this up.

> > Modified: gnucash/trunk/src/gnome/gnc-split-reg.c
> [snip]
> >    /* Now update the original with a pointer to the new one */
> > -  xaccTransBeginEdit(trans);
> >    kvp_val = kvp_value_new_guid (xaccTransGetGUID(new_trans));
> >    kvp_frame_set_slot_nc(txn_frame, "reversed-by", kvp_val);
> > -  xaccTransCommitEdit(trans);
> 
> Potentially the same thing here...

Same thing.

> > Modified: gnucash/trunk/src/gnome/reconcile-list.c
> [snip]
> > -  trans = xaccSplitGetParent(split);
> > -  xaccTransBeginEdit(trans);
> >    xaccSplitSetReconcile (split, YREC);
> >    xaccSplitSetDateReconciledSecs (split, *date);
> > -  xaccTransCommitEdit(trans);
> 
> Don't you want to transactionalize these changes into a single commit?

I don't see much reason to do so.  There aren't any uses or
documentation that would require these changes to be atomic.  If we
want to ensure such a requirement, I'd prefer to either 

a) add a check to the scrubbing code or; 

(much preferred) b) just offer an API that guarantees data
consistency, like: xaccSplitSetReconciled(split, *date) that
automatically used YREC.


> [snip]
> > -    trans = xaccSplitGetParent(split);
> > -    xaccTransBeginEdit(trans);
> >      xaccSplitSetReconcile (split, recn);
> > -    xaccTransCommitEdit(trans);
> 
> And don't we need to commit this transaction when we're done?

Yes, that happens internally to xaccSplitSetReconcile().

-chris