[gpodder-devel] Woodchuck patches
- From: neal at walfield.org (Neal H. Walfield)
- Date: Mon, 01 Aug 2011 17:51:18 +0200
Hi, Thomas,
At Mon, 1 Aug 2011 11:39:24 +0200,
Thomas Perl wrote:
> * 9d8223f626b5f7f3e30151b05304da9badc099e8 (Record the attrs that change)
>
> I don't really like this one, as it involves quite some code each time
> an attribute is set. It would be better if you implemented it so that
> it only works on save() and there it loads the data from the database
> and compares both dictionaries for changes.
Looking at the code: setting an attribute appears to be a heavy-weight
operation anyways: when an attribute is changed, save is typically
(always?) called immediately. In this case, the overhead of the
__setattr__ implementation (likely a few microseconds) is secondary
relative to the database transaction (likely tens of milliseconds).
Moreover, your suggestion adds an additional database interaction for
most (all?) attribute changes.
> This way, there's only one
> place (the save() method) where the changes are tracked, and it's easy
> to maintain and overview this piece of code, while still having the
> option to easily disable the change tracking
It's not clear to my why disabling the code in the __setattr__
function is more difficult to disable than having the code in the save
method: episode.__setattr__ = object.__setattr__.
> (I don't want to have it
> enabled by default, as it does not yet provide an advantage in the
> generic case where woodchuck isn't used).
This is reasonable although I maintain that the cost is negligible
relative to the database transaction that follows.
I'm willing to implement what you suggest, but I'm not convinced that
it is the better way. I think the __setattr__ implementation is less
complicated and less expensive than chnaging save, but, if you
disagree, I won't argue further, but will implement what you suggest.
Thoughts?
Thanks,
Neal
Other related posts:
