Merge lp:~flimm/wxbanker/none-bugfix into lp:wxbanker

[David D Lowe]

[Michael Rooney (mrooney)]

This is pretty reasonable, though I'm worried about what is causing it in the first place, as I'd not like to ignore that. However, it still seems fine to defend against. Instead of the ternary though, it is probably best to just use a simple expression like "self.Currency = currency or 0" for each.

I will happily merge this if you wouldn't mind just:
 * switching them to just using 'or'
 * add a comment above briefly mentioning why (to defend against None) and mentioned the LP #
 * add a simple test in tests/modeltests.py that creates an account by passing None as currency and balance and makes sure the object ends up with 0 and 0.0

Let me know if you need any tips or help! The comment will make it clear to anyone else looking at it later why the logic is there, and the test will make sure there isn't a regression later. It probably isn't obvious enough how to run the tests, but from the root run "python -m wxbanker/runtests". To test you won't be able to just model.CreateAccount, but instead import Account from wxbanker.bankobjects and directly instantiate.

review: Needs Fixing

[Michael Rooney (mrooney)]

PS I see you've added my Jabber so feel free to IM there as I'm usually available and IM is often good for discussion back and forth.

[Michael Rooney (mrooney)]

Sorry for the delay, looks great, I'll merge this in now :]

review: Approve

=== modified file 'wxbanker/bankobjects/account.py'
--- wxbanker/bankobjects/account.py	2010-08-09 00:31:45 +0000
+++ wxbanker/bankobjects/account.py	2011-06-01 17:56:28 +0000
@@ -41,8 +41,9 @@
         self._Transactions = None
         self._RecurringTransactions = []
         self._preTransactions = []
-        self.Currency = currency
-        self.Balance = balance
+        # Make sure that Currency and Balance are not None (bug #653716)
+        self.Currency = currency or 0
+        self.Balance = balance or 0.0
         self.MintId = mintId
         self.IsFrozen = False
 
@@ -350,4 +351,4 @@
     RecurringTransactions = property(GetRecurringTransactions)
     Currency = property(GetCurrency, SetCurrency)
     MintId = property(GetMintId, SetMintId)
-    CurrentBalance = property(GetCurrentBalance)
 \ No newline at end of file
+    CurrentBalance = property(GetCurrentBalance)
 
=== modified file 'wxbanker/tests/modeltests.py'
--- wxbanker/tests/modeltests.py	2010-08-09 00:31:45 +0000
+++ wxbanker/tests/modeltests.py	2011-06-01 17:56:28 +0000
@@ -22,6 +22,7 @@
 import os, datetime, unittest
 from wxbanker import controller, bankexceptions, currencies
 from wx.lib.pubsub import Publisher
+from wxbanker.bankobjects.account import Account
 
 from wxbanker.tests.testbase import today, yesterday, tomorrow
 
@@ -541,6 +542,20 @@
         self.assertEqual(a.Balance, 2)
         self.assertEqual(a.CurrentBalance, 1)
         
+    def testAccountBalanceAndCurrencyNotNone(self):
+        model = self.Model
+        accounts = [
+            Account(model, None, "Foo"),
+            Account(model, None, "Bar", currency=None),
+            Account(model, None, "Food", balance=None),
+            Account(model, None, "Pub", currency=None, balance=None)
+        ]
+        
+        for account in accounts:
+            self.assertEqual(account.Currency, currencies.CurrencyList[0]())
+            self.assertEqual(account.Balance, 0.0)
+            self.assertTrue(type(account.Balance) is float)
+                
  
         
 if __name__ == "__main__":