Cleaned up the way that commodity pricing is handled.

5 files changed, 64 insertions, 153 deletions

diff --git a/src/amount.h b/src/amount.h
index 006c554a..c6d2c27c 100644
--- a/src/amount.h
+++ b/src/amount.h
@@ -241,6 +241,12 @@ public:
     else
       commodity_ = NULL;
   }
+  amount_t(const amount_t& amt, const annotation_t& details) : quantity(NULL) {
+    TRACE_CTOR(amount_t, "const amount_t&, const annotation_t&");
+    assert(amt.quantity);
+    _copy(amt);
+    annotate(details);
+  }
   amount_t& operator=(const amount_t& amt);
 
 #ifdef HAVE_GDTOA
diff --git a/src/commodity.cc b/src/commodity.cc
index 01f14ac9..29bb2f56 100644
--- a/src/commodity.cc
+++ b/src/commodity.cc
@@ -43,8 +43,7 @@
 
 namespace ledger {
 
-void commodity_t::add_price(const datetime_t& date,
-			    const amount_t& price)
+void commodity_t::add_price(const datetime_t& date, const amount_t& price)
 {
   if (! base->history)
     base->history = history_t();
@@ -117,19 +116,26 @@ optional<amount_t> commodity_t::value(const optional<datetime_t>& moment)
   return price;
 }
 
-amount_t commodity_t::exchange(const amount_t&		   amount,
-			       amount_t&		   final_cost, // out
-			       amount_t&		   basis_cost, // out
-			       const optional<amount_t>&   total_cost_,
-			       const optional<amount_t>&   per_unit_cost_,
-			       const optional<datetime_t>& moment,
-			       const optional<string>&     tag)
+void commodity_t::exchange(commodity_t&	     commodity,
+			   const amount_t&   per_unit_cost,
+			   const datetime_t& moment)
 {
-  // (assert (or (and total-cost (not per-unit-cost))
-  //             (and per-unit-cost (not total-cost))))
+  if (! commodity.has_flags(COMMODITY_STYLE_NOMARKET)) {
+    commodity_t& base_commodity
+      (commodity.annotated ?
+       as_annotated_commodity(commodity).referent() : commodity);
 
-  assert((total_cost_ && ! per_unit_cost_) || (per_unit_cost_ && ! total_cost_));
+    base_commodity.add_price(moment, per_unit_cost);
+  }
+}
 
+commodity_t::cost_breakdown_t
+commodity_t::exchange(const amount_t&		  amount,
+		      const amount_t&		  cost,
+		      const bool		  is_per_unit,
+		      const optional<datetime_t>& moment,
+		      const optional<string>&     tag)
+{
   // (let* ((commodity (amount-commodity amount))
   //        (current-annotation
   //         (and (annotated-commodity-p commodity)
@@ -152,9 +158,10 @@ amount_t commodity_t::exchange(const amount_t&		   amount,
     (current_annotation ?
      as_annotated_commodity(commodity).referent() : commodity);
 
-  amount_t per_unit_cost(per_unit_cost_ ?
-			 *per_unit_cost_ : *total_cost_ / amount);
-  final_cost = total_cost_ ? *total_cost_ : *per_unit_cost_ * amount;
+  amount_t per_unit_cost(is_per_unit ? cost : cost / amount);
+
+  cost_breakdown_t breakdown;
+  breakdown.final_cost = ! is_per_unit ? cost : cost * amount;
 
   // Add a price history entry for this conversion if we know when it took
   // place
@@ -177,16 +184,15 @@ amount_t commodity_t::exchange(const amount_t&		   amount,
   //             total-cost))))
 
   if (current_annotation && current_annotation->price)
-    basis_cost = *current_annotation->price * amount;
+    breakdown.basis_cost = *current_annotation->price * amount;
   else
-    basis_cost = final_cost;
+    breakdown.basis_cost = breakdown.final_cost;
 
-  amount_t ann_amount(amount);
-  ann_amount.annotate
-    (annotation_t(per_unit_cost,
-		  moment ? moment->date() : optional<date_t>(), tag));
+  breakdown.amount =
+    amount_t(amount, annotation_t (per_unit_cost, moment ?
+				   moment->date() : optional<date_t>(), tag));
 
-  return ann_amount;
+  return breakdown;
 }
 
 commodity_t::operator bool() const
diff --git a/src/commodity.h b/src/commodity.h
index 27827c1a..af90f6b8 100644
--- a/src/commodity.h
+++ b/src/commodity.h
@@ -192,18 +192,26 @@ public:
     return base->history;
   }
 
-  void	   add_price(const datetime_t& date, const amount_t& price);
-  bool	   remove_price(const datetime_t& date);
+  void add_price(const datetime_t& date, const amount_t& price);
+  bool remove_price(const datetime_t& date);
 
   optional<amount_t> value(const optional<datetime_t>& moment = none);
 
-  static amount_t exchange(const amount_t&	       amount,
-			   amount_t&		       final_cost, // out
-			   amount_t&		       basis_cost, // out
-			   const optional<amount_t>&   total_cost,
-			   const optional<amount_t>&   per_unit_cost = none,
-			   const optional<datetime_t>& moment        = none,
-			   const optional<string>&     tag           = none);
+  struct cost_breakdown_t {
+    amount_t amount;
+    amount_t final_cost;
+    amount_t basis_cost;
+  };
+
+  static void             exchange(commodity_t&	     commodity,
+				   const amount_t&   per_unit_cost,
+				   const datetime_t& moment);
+
+  static cost_breakdown_t exchange(const amount_t&	       amount,
+				   const amount_t&	       cost,
+				   const bool		       is_per_unit = false,
+				   const optional<datetime_t>& moment	   = none,
+				   const optional<string>&     tag	   = none);
 
   static void parse_symbol(std::istream& in, string& symbol);
   static void parse_symbol(char *& p, string& symbol);
diff --git a/src/entry.cc b/src/entry.cc
index 5bb55b39..cfed75a9 100644
--- a/src/entry.cc
+++ b/src/entry.cc
@@ -98,19 +98,6 @@ bool entry_base_t::finalize()
   value_t  balance;
   xact_t * null_xact = NULL;
 
-  //   (do-xacts (xact entry)
-  //     (when (xact-must-balance-p xact)
-  //       (let ((amt (xact-amount* xact)))
-  //         (if amt
-  //             (setf balance (add balance (or (xact-cost xact) amt)))
-  //             (if null-xact
-  //                 (error "Only one xact with null amount allowed ~
-  //                         per entry (beg ~S end ~S)"
-  //                        (item-position-begin-line (entry-position entry))
-  //                        (item-position-end-line (entry-position entry)))
-  //                 (setf null-xact xact))))))
-  //
-
   foreach (xact_t * xact, xacts) {
     if (xact->must_balance()) {
       amount_t& p(xact->cost ? *xact->cost : xact->amount);
@@ -133,17 +120,6 @@ bool entry_base_t::finalize()
   // If there is only one xact, balance against the default account if
   // one has been set.
 
-  //   (when (= 1 (length (entry-xacts entry)))
-  //     (if-let ((default-account
-  //                  (journal-default-account (entry-journal entry))))
-  //       (setf null-xact
-  //             (make-xact :entry entry
-  //                               :status (xact-status
-  //                                        (first (entry-xacts entry)))
-  //                               :account default-account
-  //                               :generatedp t))
-  //       (add-xact entry null-xact)))
-
   if (journal && journal->basket && xacts.size() == 1 && ! balance.is_null()) {
     // jww (2008-07-24): Need to make the rest of the code aware of what to do
     // when it sees a generated xact.
@@ -157,24 +133,6 @@ bool entry_base_t::finalize()
     // inverse of the rest.  If multiple commodities are involved, multiple
     // xacts are generated to balance them all.
 
-    // (progn
-    //   (if (balance-p balance)
-    //       (let ((first t))
-    //         (dolist (amount (balance-amounts balance))
-    //           (if first
-    //               (setf (xact-amount* null-xact) (negate amount)
-    //                     first nil)
-    //               (add-xact
-    //                entry
-    //                (make-xact :entry entry
-    //                                  :account (xact-account null-xact)
-    //                                  :amount (negate amount)
-    //                                  :generatedp t)))))
-    //       (setf (xact-amount* null-xact) (negate balance)
-    //             (xact-calculatedp null-xact) t))
-    //
-    //   (setf balance 0))
-
     if (balance.is_balance()) {
       bool first = true;
       const balance_t& bal(balance.as_balance());
@@ -206,21 +164,6 @@ bool entry_base_t::finalize()
     // establishes the per-unit cost for this xact for both
     // commodities.
 
-    // (when (and (balance-p balance)
-    //            (= 2 (balance-commodity-count balance)))
-    //   (destructuring-bind (x y) (balance-amounts balance)
-    //     (let ((a-commodity (amount-commodity x))
-    //           (per-unit-cost (value-abs (divide x y))))
-    //       (do-xacts (xact entry)
-    //         (let ((amount (xact-amount* xact)))
-    //           (unless (or (xact-cost xact)
-    //                       (not (xact-must-balance-p xact))
-    //                       (commodity-equal (amount-commodity amount)
-    //                                        a-commodity))
-    //             (setf balance (subtract balance amount)
-    //                   (xact-cost xact) (multiply per-unit-cost amount)
-    //                   balance (add balance (xact-cost xact))))))))))
-
     const balance_t& bal(balance.as_balance());
 
     balance_t::amounts_map::const_iterator a = bal.amounts.begin();
@@ -234,22 +177,13 @@ bool entry_base_t::finalize()
       commodity_t& comm(x.commodity());
 
       foreach (xact_t * xact, xacts) {
-	const amount_t& x_amt(xact->amount);
-
-	if (! (xact->cost ||
-	       ! xact->must_balance() ||
-	       x_amt.commodity() == comm)) {
-	  DEBUG("entry.finalize", "before operation 1 = " << balance);
-	  balance -= x_amt;
-	  DEBUG("entry.finalize", "after operation 1 = " << balance);
-	  DEBUG("entry.finalize", "x_amt = " << x_amt);
-	  DEBUG("entry.finalize", "per_unit_cost = " << per_unit_cost);
-
-	  xact->cost = per_unit_cost * x_amt;
-	  DEBUG("entry.finalize", "*xact->cost = " << *xact->cost);
+	const amount_t& amt(xact->amount);
 
+	if (! (xact->cost || ! xact->must_balance() ||
+	       amt.commodity() == comm)) {
+	  balance -= amt;
+	  xact->cost = per_unit_cost * amt;
 	  balance += *xact->cost;
-	  DEBUG("entry.finalize", "after operation 2 = " << balance);
 	}
 
       }
@@ -262,58 +196,23 @@ bool entry_base_t::finalize()
   // once more in terms of total cost, accounting for any possible gain/loss
   // amounts.
 
-  // (do-xacts (xact entry)
-  //   (when (xact-cost xact)
-  //     (let ((amount (xact-amount* xact)))
-  //       (assert (not (commodity-equal (amount-commodity amount)
-  //                                     (amount-commodity (xact-cost xact)))))
-  //       (multiple-value-bind (annotated-amount total-cost basis-cost)
-  //           (exchange-commodity amount :total-cost (xact-cost xact)
-  //                               :moment (entry-date entry)
-  //                               :tag (entry-code entry))
-  //         (if (annotated-commodity-p (amount-commodity amount))
-  //             (if-let ((price (annotation-price
-  //                              (commodity-annotation
-  //                               (amount-commodity amount)))))
-  //               (setf balance
-  //                     (add balance (subtract basis-cost total-cost))))
-  //             (setf (xact-amount* xact) annotated-amount))))))
-
   foreach (xact_t * xact, xacts) {
     if (xact->cost) {
-      const amount_t& x_amt(xact->amount);
+      if (xact->amount.commodity() == xact->cost->commodity())
+	throw_(balance_error, "Transaction's cost must be of a different commodity");
 
-      assert(x_amt.commodity() != xact->cost->commodity());
+      commodity_t::cost_breakdown_t breakdown =
+	commodity_t::exchange(xact->amount, *xact->cost);
 
-      entry_t * entry = dynamic_cast<entry_t *>(this);
-
-      // jww (2008-07-24): Pass the entry's code here if we can, as the
-      // auto-tag
-      amount_t final_cost;
-      amount_t basis_cost;
-      amount_t ann_amount =
-	commodity_t::exchange(x_amt, final_cost, basis_cost, xact->cost);
-
-      if (xact->amount.is_annotated()) {
-	if (ann_amount.annotation().price)
-	  add_or_set_value(balance, basis_cost - final_cost);
-      } else {
-	xact->amount = ann_amount;
-      }
+      if (xact->amount.is_annotated())
+	add_or_set_value(balance, breakdown.basis_cost - breakdown.final_cost);
+      else
+	xact->amount = breakdown.amount;
     }
   }
 
   DEBUG("entry.finalize", "final balance = " << balance);
 
-  // (if (value-zerop balance)
-  //     (prog1
-  //         entry
-  //       (setf (entry-normalizedp entry) t))
-  //     (error "Entry does not balance (beg ~S end ~S); remaining balance is:~%~A"
-  //            (item-position-begin-line (entry-position entry))
-  //            (item-position-end-line (entry-position entry))
-  //            (format-value balance :width 20)))
-
   if (! balance.is_null()) {
     balance.in_place_round();
     if (! balance.is_zero()) {
diff --git a/src/journal.cc b/src/journal.cc
index c5a9e9fc..63b99e6f 100644
--- a/src/journal.cc
+++ b/src/journal.cc
@@ -101,14 +101,6 @@ bool journal_t::add_entry(entry_t * entry)
 
   entries.push_back(entry);
 
-  foreach (const xact_t * xact, entry->xacts)
-    if (xact->cost) {
-      assert(xact->amount);
-      xact->amount.commodity().add_price(datetime_t(*entry->date(),
-						    time_duration_t(0, 0, 0)),
-					 *xact->cost / xact->amount.number());
-    }
-
   return true;
 }