[patch 7/8] Remove xaccAccountRemoveSplit, and xaccAccountInsertSplit.

c.shoemaker at cox.net c.shoemaker at cox.net
Thu Mar 2 00:08:16 EST 2006


   Remove xaccAccountRemoveSplit, and xaccAccountInsertSplit.
   (Actually, xaccAccountInsertSplit is #def'd to xaccSplitSetAccount().)
   Accounts no longer manage their Split lists.  The Account split lists
   should only ever be modified from within xaccTransCommitEdit().  This
   simplifies some of the operations on Accounts, since they can now be 
   expressed in terms of operations on Splits.

   Also, the consolidation of two operations (RemoveSplit and InsertSplit) 
   into one operation (xaccSplitSetAccount) means that it's no longer 
   possible to orphan a Split.

   Incidental:
      Fix a leak of a KVP frame.
      Don't recompute the account balance if we're shutting down.

---
 src/engine/Account.c  |  244 ++++++--------------------------------------------
 src/engine/Account.h  |   11 --
 src/engine/AccountP.h |    9 -
 3 files changed, 33 insertions(+), 231 deletions(-)

Index: trunk/src/engine/Account.c
===================================================================
--- trunk.orig/src/engine/Account.c
+++ trunk/src/engine/Account.c
@@ -143,6 +143,7 @@ xaccCloneAccountCommon(const Account *fr
     ret->accountCode = CACHE_INSERT(from->accountCode);
     ret->description = CACHE_INSERT(from->description);
 
+    kvp_frame_delete(ret->inst.kvp_data);
     ret->inst.kvp_data = kvp_frame_copy(from->inst.kvp_data);
 
     /* The new book should contain a commodity that matches
@@ -161,6 +162,8 @@ xaccCloneAccount (const Account *from, Q
 {
     Account *ret = xaccCloneAccountCommon(from, book);
     qof_instance_gemini (&ret->inst, (QofInstance *) &from->inst);
+    g_assert (ret ==
+              (Account*) qof_instance_lookup_twin (QOF_INSTANCE(from), book));
     return ret;
 }
 
@@ -178,7 +181,6 @@ xaccCloneAccountSimple (const Account *f
 void
 xaccFreeAccount (Account *acc)
 {
-  Transaction *t;
   GList *lp;
 
   if (!acc) return;
@@ -216,29 +218,20 @@ xaccFreeAccount (Account *acc)
    * check once we know the warning isn't occurring any more. */
   if (acc->splits) 
   {
+    GList *slist;
     PERR (" instead of calling xaccFreeAccount(), please call \n"
           " xaccAccountBeginEdit(); xaccAccountDestroy(); \n");
   
-    /* any split pointing at this account needs to be unmarked */
-    for(lp = acc->splits; lp; lp = lp->next) 
-    {
-      Split *s = lp->data;
-      s->acc = NULL;
-    }
-  
     acc->inst.editlevel = 0;
-  
-    for(lp = acc->splits; lp; lp = lp->next) {
+
+    slist = g_list_copy(acc->splits);
+    for (lp = slist; lp; lp = lp->next) {
       Split *s = (Split *) lp->data;
-      t = s->parent;
-      xaccTransBeginEdit (t);
+      g_assert(xaccSplitGetAccount(s) == acc);
       xaccSplitDestroy (s);
-      xaccTransCommitEdit (t);
     }
-  
-    /* free up array of split pointers */
-    g_list_free(acc->splits);
-    acc->splits = NULL;
+    g_list_free(slist);
+    g_assert(acc->splits == NULL);
   }
 
   CACHE_REPLACE(acc->accountName, NULL);
@@ -301,7 +294,7 @@ xaccAccountCommitEdit (Account *acc) 
    * and then the splits ... */
   if (acc->inst.do_free)
   {
-    GList *lp;
+    GList *lp, *slist;
  
     acc->inst.editlevel++;
 
@@ -312,16 +305,14 @@ xaccAccountCommitEdit (Account *acc) 
     PINFO ("freeing splits for account %p (%s)",
            acc, acc->accountName ? acc->accountName : "(null)");
 
-    while (acc->splits)
+    slist = g_list_copy(acc->splits);
+    for (lp = slist; lp; lp = lp->next)
     {
-      Split *s = acc->splits->data;
-      Transaction *t = s->parent;
-
-      xaccTransBeginEdit (t);
+      Split *s = lp->data;
       xaccSplitDestroy (s);
-      xaccTransCommitEdit (t);
     }
-
+    g_assert(acc->splits == NULL || qof_book_shutting_down(acc->inst.book));
+    g_list_free(slist);
     /* the lots should be empty by now */
     for (lp=acc->lots; lp; lp=lp->next)
     {
@@ -584,22 +575,12 @@ xaccAccountEqual(const Account *aa, cons
 
 /********************************************************************\
 \********************************************************************/
-
-static gint
-split_sort_func(gconstpointer a, gconstpointer b) {
-  /* don't coerce xaccSplitDateOrder so we'll catch changes */
-  /* huh?  what changes? */
-  Split *sa = (Split *) a;
-  Split *sb = (Split *) b;
-  return(xaccSplitDateOrder(sa, sb));
-}
-
 void
 xaccAccountSortSplits (Account *acc, gboolean force)
 {
   if (!acc || !acc->sort_dirty || (!force && acc->inst.editlevel > 0)) return;
 
-  acc->splits = g_list_sort(acc->splits, split_sort_func);
+  acc->splits = g_list_sort(acc->splits, (GCompareFunc)xaccSplitDateOrder);
   acc->sort_dirty = FALSE;
   acc->balance_dirty = TRUE;
 }
@@ -772,105 +753,6 @@ xaccAccountInsertLot (Account *acc, GNCL
 
 /********************************************************************\
 \********************************************************************/
-
-void
-xaccAccountInsertSplit (Account *acc, Split *split)
-{
-  Transaction *trans;
-
-  if (!acc || !split || split->acc == acc) return;
-  /* check for book mix-up */
-  g_return_if_fail (acc->inst.book == split->inst.book);
-
-  trans = xaccSplitGetParent (split);
-  ENTER ("(acc=%p, trans=%p, split=%p)", acc, trans, split);
-
-  if (trans)
-      xaccTransBeginEdit(trans);
-
-
-  /* If this split belongs to another account, remove it from there
-   * first.  We don't want to ever leave the system in an inconsistent
-   * state.  */
-  xaccAccountRemoveSplit(split->acc, split);
-
-  split->acc = acc;  /* BUG? Doesn't actually dirty split. */
-
-  /* If the split's lot belonged to some other account, we leave it so. */
-  if (split->lot && (NULL == split->lot->account))
-      xaccAccountInsertLot (acc, split->lot);
-
-  if (!g_list_find(acc->splits, split)) {
-      if (acc->inst.editlevel == 0)
-      {
-          acc->splits = g_list_insert_sorted(acc->splits, split,
-                                             split_sort_func);
-      }
-      else
-      {
-          acc->splits = g_list_prepend(acc->splits, split);
-          acc->sort_dirty = TRUE;
-      }
-
-      acc->balance_dirty = TRUE;
-      mark_account (acc);
-  }
-
-  /* Setting the amount causes a conversion to the new account's
-   * denominator AKA 'SCU Smallest Currency Unit'. */
-  xaccSplitSetAmount(split, xaccSplitGetAmount(split));
-
-  if (trans)
-      xaccTransCommitEdit(trans);
-
-  LEAVE ("(acc=%p, trans=%p, split=%p)", acc, trans, split);
-}
-
-/********************************************************************\
-\********************************************************************/
-
-void
-xaccAccountRemoveSplit (Account *acc, Split *split)
-{
-  GList *node;
-  if (!acc || !split || split->acc != acc) return;
-
-  ENTER ("(acc=%p, split=%p)", acc, split);
-
-  
-  node = g_list_find (acc->splits, split);
-  if (node) {
-      Transaction *trans = xaccSplitGetParent (split);
-
-      acc->splits = g_list_delete_link (acc->splits, node);
-
-      acc->balance_dirty = TRUE;
-      mark_account (acc);
-
-      xaccTransBeginEdit (trans);
-      split->acc = NULL;
-
-      /* Remove from lot (but only if it hasn't been moved to new lot
-         already) */
-      if (split->lot && split->lot->account == acc)
-          gnc_lot_remove_split (split->lot, split);
-      
-      xaccTransCommitEdit (trans);
-
-      /* BUG? Umm... trans IS the split parent so didn't we just
-         generate the event when we called CommitEdit? I think this
-         gen_event can be removed. */
-      if (split->parent)
-        gnc_engine_gen_event (&split->parent->inst.entity, GNC_EVENT_MODIFY);
-
-  } else PERR ("split is not in account, but it thinks it is!");
-  
-  LEAVE ("(acc=%p, split=%p)", acc, split);
-}
-
-/********************************************************************\
-\********************************************************************/
-
 static void
 xaccPreSplitMove (Split *split, gpointer dummy)
 {
@@ -882,15 +764,10 @@ xaccPostSplitMove (Split *split, Account
 {
   Transaction *trans;
 
-  split->acc = accto;
-  /* Better? xaccSplitSetAmount(split, split->ammount); */
-  split->amount = gnc_numeric_convert (split->amount,
-				       xaccAccountGetCommoditySCU(accto),
-				       GNC_HOW_RND_ROUND);
+  xaccSplitSetAccount(split, accto);
+  xaccSplitSetAmount(split, split->amount);
   trans = xaccSplitGetParent (split);
   xaccTransCommitEdit (trans);
-  /* BUG? again, extra events? */
-  gnc_engine_gen_event (&trans->inst.entity, GNC_EVENT_MODIFY);
 }
 
 void
@@ -903,18 +780,20 @@ xaccAccountMoveAllSplits (Account *accfr
   g_return_if_fail (accfrom->inst.book == accto->inst.book);
   ENTER ("(accfrom=%p, accto=%p)", accfrom, accto);
 
+  xaccAccountBeginEdit(accfrom);
+  xaccAccountBeginEdit(accto);
   /* Begin editing both accounts and all transactions in accfrom. */
   g_list_foreach(accfrom->splits, (GFunc)xaccPreSplitMove, NULL);
 
   /* Concatenate accfrom's lists of splits and lots to accto's lists. */
-  accto->splits = g_list_concat(accto->splits, accfrom->splits);
-  accto->lots = g_list_concat(accto->lots, accfrom->lots);
+  //accto->splits = g_list_concat(accto->splits, accfrom->splits);
+  //accto->lots = g_list_concat(accto->lots, accfrom->lots);
 
   /* Set appropriate flags. */
-  accfrom->balance_dirty = TRUE;
-  accfrom->sort_dirty = FALSE;
-  accto->balance_dirty = TRUE;
-  accto->sort_dirty = TRUE;
+  //accfrom->balance_dirty = TRUE;
+  //accfrom->sort_dirty = FALSE;
+  //accto->balance_dirty = TRUE;
+  //accto->sort_dirty = TRUE;
 
   /*
    * Change each split's account back pointer to accto.
@@ -924,18 +803,10 @@ xaccAccountMoveAllSplits (Account *accfr
   g_list_foreach(accfrom->splits, (GFunc)xaccPostSplitMove, (gpointer)accto);
 
   /* Finally empty accfrom. */
-  accfrom->splits = NULL;
-  accfrom->lots = NULL;
-
-  /*
-   * DNJ - I don't really understand why this is necessary,
-   *       but xaccAccountInsertSplit does it.
-   */
-  if (accto->inst.editlevel == 0)
-  {
-    accto->splits = g_list_sort(accto->splits, split_sort_func);
-    accto->sort_dirty = FALSE;
-  }
+  g_assert(accfrom->splits == NULL);
+  g_assert(accfrom->lots == NULL);
+  xaccAccountCommitEdit(accfrom);
+  xaccAccountCommitEdit(accto);
 
   LEAVE ("(accfrom=%p, accto=%p)", accfrom, accto);
 }
@@ -982,6 +853,7 @@ xaccAccountRecomputeBalance (Account * a
   if (acc->inst.editlevel > 0) return;
   if (!acc->balance_dirty) return;
   if (acc->inst.do_free) return;
+  if (qof_book_shutting_down(acc->inst.book)) return;
 
   balance            = acc->starting_balance;
   cleared_balance    = acc->starting_cleared_balance;
@@ -1042,58 +914,6 @@ xaccAccountSetStartingBalance(Account *a
 }
 
 /********************************************************************\
- * xaccAccountFixSplitDateOrder                                     *
- *   check this split to see if the date is in correct order        *
- *   If it is not, reorder the transactions ...                     *
- *                                                                  *
- * Args:   acc   -- the account to check                            *
- *         split -- the split to check                              *
- *                                                                  *
- * Return: int -- non-zero if out of order                          *
-\********************************************************************/
-/* CAS: Umm, we say we're checking the split, but we're actually
-   resorting all the splits.  Why not just leave the split out of
-   it? FIXME. */
-void
-xaccAccountFixSplitDateOrder (Account * acc, Split *split) 
-{
-  if (!acc || !split || acc->inst.do_free) return;
-
-  /* FIXME: This looks wrong, too.  Why force it dirty if it's not? */
-  acc->sort_dirty = TRUE;
-  acc->balance_dirty = TRUE;
-
-  if (acc->inst.editlevel <= 0)
-      xaccAccountBringUpToDate (acc);
-}
-
-/********************************************************************\
- * xaccTransFixSplitDateOrder                                       *
- *   check this transaction to see if the date is in correct order  *
- *   If it is not, reorder the transactions.                        *
- *   This routine perfroms the check for both of the double-entry   *
- *   transaction entries.                                           *
- *                                                                  *
- * Args:   trans -- the transaction to check                        *
-\********************************************************************/
-
-void
-xaccTransFixSplitDateOrder (Transaction *trans)
-{
-  GList *node;
-
-  if (!trans) return;
-
-  gnc_engine_suspend_events();
-  for (node = trans->splits; node; node = node->next)
-  {
-    Split *s = node->data;
-    xaccAccountFixSplitDateOrder (s->acc, s);
-  }
-  gnc_engine_resume_events();
-}
-
-/********************************************************************\
 \********************************************************************/
 
 /* The sort order is used to implicitly define an 
Index: trunk/src/engine/Account.h
===================================================================
--- trunk.orig/src/engine/Account.h
+++ trunk/src/engine/Account.h
@@ -511,7 +511,7 @@ guint32 xaccAccountTypesValid(void);
  *    split into the indicated account.  If the split already 
  *    belongs to another account, it will be removed from that
  *    account first.*/
-void xaccAccountInsertSplit (Account *account, Split *split);
+#define xaccAccountInsertSplit(acc, s)  xaccSplitSetAccount((s), (acc))
 
 /** The xaccAccountGetSplitList() routine returns a pointer to a GList of
  *    the splits in the account.  
@@ -566,15 +566,6 @@ Transaction * xaccAccountFindTransByDesc
 Split * xaccAccountFindSplitByDesc(const Account *account, 
                                    const char *description);
 
-/** The xaccAccountFixSplitDateOrder() subroutine checks to see if 
- *    a split is in proper sorted date order with respect 
- *    to the other splits in this account. */
-void xaccAccountFixSplitDateOrder (Account *account, Split *split);
-
-/** The xaccTransFixSplitDateOrder() checks to see if 
- *    all of the splits in this transaction are in
- *    proper date order. */
-void xaccTransFixSplitDateOrder (Transaction *trans);
 /*@}*/
 
 /* ------------------ */
Index: trunk/src/engine/AccountP.h
===================================================================
--- trunk.orig/src/engine/AccountP.h
+++ trunk/src/engine/AccountP.h
@@ -133,15 +133,6 @@ struct account_s
   guint32  idata;     /* used by the sql backend for kvp management */
 };
 
-/* The xaccAccountRemoveSplit() routine will remove the indicated split
- *    from the indicated account.  Note that this will leave the split
- *    "dangling", i.e. unassigned to any account, and therefore will put
- *    the engine into an inconsistent state.  After removing a split, 
- *    it should be immediately destroyed, or it should be inserted into  
- *    an account.
- */
-void xaccAccountRemoveSplit (Account *, Split *);
-
 /* The xaccAccountSortSplits() routine will resort the account's 
  * splits if the sort is dirty. If 'force' is true, the account 
  * is sorted even if the editlevel is not zero. 

--


More information about the gnucash-devel mailing list