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
(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
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
def isSyncronized(self):
if not (hasattr(self, "prog_name") and hasattr(self, "sync_file")):
return SyncType.NOT_ENABLED
...
class SyncType:
NO_SYNC = 0x00
UNDER_DATE = 0x01
NO_FILE = 0x02
SYNC = 0x04
NOT_ENABLED = 0x08
values = {
value1: do_some_stuff1,
value2: do_some_stuff2,
...
valueN: do_some_stuffN,
}
values.get(var, do_default_stuff)()
wapcaplet.myopenid.com
October 31, 2010, October 31, 2010 05:09, permalink
Just a minor suggestion: you could remove the first "else:" and unindent that whole block. I tend to do this whenever there's an "if" statement that returns from the function, since none of the code below it will be reached anyway:
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
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
I wanna know if anyone can make this block a little more readable