-
Notifications
You must be signed in to change notification settings - Fork 309
Conversation
DRY out the BalancedThings.
|
||
_customer = None # underlying balanced.Customer object | ||
_thing = None # underlying balanced.{BankAccount,Card} object | ||
|
||
def _get(self, name, default=""): | ||
def __getitem__(self, key, default=""): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm bad at Python, but did you mean to change this? was this part of the bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's incidental to the bug, which was introduced in BalancedCard.__getitem__
. Simplifying the way things are accessed in BalancedThing
s reduces the chance of this happening in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What @grampajoe said.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how getitem works with 3 parameters. I think it is supposed to have only 2. See http://docs.python.org/2/reference/datamodel.html#object.__getitem__
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Removed in c1cc22a.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P.S. Removing the default
argument and returning None
in the default case led to #2161.
Holdover from when it was _get.
#2111