On 9/3/2013 12:07 PM, Gildor Oronar wrote:
What would you choose? Do you put logging routine into caller or callee?
My intuitive answer is "callee does the logging, because that's where
action takes place", like this:
class Account():
def transaction(self, amount, target):
logging.info("Start transaction of %s to %s" % (amount, target))
...
if '__name__' == '__main__'
....
account.transaction(amount, target)
Simple and easy. Put logging code to the caller would be tedious - the
function is called about 20 times in different places.
So far so good, but we grew up to have 10 different account classes:
class AbsctractAccount():
class CreditAccount(AbstractAccount):
def transaction(self, amount, target):
logging.info("Start transaction of %s to %s" % (amount, target))
...
class DebitAccount(AbstractAccount):
def transaction(self, amount, target):
logging.info("Start transaction of %s to %s" % (amount, target))
...
class SomeOtherAccount(...)
....
Then letting the callee do the logging is also tedious now.
What is the best practise here?
If, for the convenience, we define transaction function in
AbstractAccount to just do the logging, and change inherited classes,
like this:
class AbsctractAccount():
def transaction(self, amount, target):
logging.info("Start transaction of %s to %s" % (amount, target))
class DebitAccount(AbstractAccount):
def transaction(self, amount, target):
super().transaction(amount,target)
....
Then we gethered logging code pieces all to one place, but it begets two
other problems.
1. It is a "surprise" that super().transaction must be called first,
Not to me ;-). That is fairly standard in super examples I have seen posted.
> not last, and that it does only the logging.
If that is the only thing in common ...
2. The code used to check whether "transaction" is implemented:
> if hasAttr(DebitAccount, 'transaction'): # clear to read
has to be changed to check whether "transaction" is inherited:
if not DebitAccount.transaction is AbstractAccount.transaction:
whose purpose is confusing, and whose cause is a little surprise too.
I would expect that every account class has a transaction method.
* If so, just call it, but
assertIsNot(DebitAccount.transaction, AbstractAccount.transaction)
for every subclass in your test suite.
* If not, perhaps you need an abstract subclass TransAccount. Then use
hasattr in production code and the isnot test in test code.
Also, the pythonic "boldly calling and watching for exception" stopped
working, because instead of getting AttributeError, we would get a line
of log and nothing more.
This is what test suites are for.
--
Terry Jan Reedy
--
https://mail.python.org/mailman/listinfo/python-list
