1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23
def isSyncronized(self): NO_SYNC = 1 UNDER_DATE = 2 NO_FILE = 3 SYNC = 4 if not (hasattr(self, "prog_name") and hasattr(self, "sync_file")): return False else: database = PluginBase.database if not database.record_exists(self.prog_name): return NO_SYNC record = database.get_record(self.prog_name) record_mtime = record["mtime"] if not os.path.isfile(self.sync_file): return NO_FILE file_mtime = os.path.getmtime(self.sync_file) if file_mtime > record_mtime: return SYNC return UNDER_DAT
Refactorings
No refactoring yet !
Tony
August 5, 2010, August 05, 2010 01:54, permalink
No rating.
Login to rate!
(Whoops, I accidentally gave the wrong values to NO_SYNC, UNDER_DATE, NO_FILE, SYNC)
Peter Jensen
August 6, 2010, August 06, 2010 20:28, permalink
No rating.
Login to rate!
I'd drop the return False; and have a SyncType class - with the addition of SyncType.NOT_ENABLED instead of the False return.
Note that:
if isSyncronized():
pass
Fires for everything except the return of False. Having a class similar to an enumerated type forces the code to be somewhat self documenting.
Otherwise I'm not sure the "ifs" are too big of a deal. You might consider trying
http://www.mustap.com/pythonzone_post_224_python-switch-statement
-Pete
Function
1 2 3 4 5
def isSyncronized(self): if not (hasattr(self, "prog_name") and hasattr(self, "sync_file")): return SyncType.NOT_ENABLED ...
SyncType Class
1 2 3 4 5 6
class SyncType: NO_SYNC = 0x00 UNDER_DATE = 0x01 NO_FILE = 0x02 SYNC = 0x04 NOT_ENABLED = 0x08
Pythonic Switch
1 2 3 4 5 6 7 8 9 10 11 12 13 14
values = {
value1: do_some_stuff1,
value2: do_some_stuff2,
...
valueN: do_some_stuffN,
}
values.get(var, do_default_stuff)()
I wanna know if anyone can make this block a little more readable