1 2 3 4 5 6 7 8 9 10 11 12 13
class Planet(object): all = [] @classmethod def get(self, **kwargs): for p in self.all: match = True for key, value in kwargs.items(): if p.__dict__[key] != value: match = False print match if match: return p return None
Refactorings
No refactoring yet !
micheal d.
September 8, 2008, September 08, 2008 18:05, permalink
Quite frankly, i would like you to say more what it should do, but i presume 'all' is a list of dict-like objects.
1 2 3 4 5 6 7 8 9 10 11 12 13
class Planet(object): def __init__(self, all = []) self.all = all @classmethod def get(self, **kwargs) for elem in self.all: for key, value in kwargs.items(): match = elem.__dict__.get(key, not value) != value if match: print match, p else: print False return None
Patch
September 8, 2008, September 08, 2008 20:02, permalink
List comprehensions are _sometimes_ faster (and more aesthetically pleasing), but it seems like it would be a lot less space efficient in this case. It also makes printing a bit ugly.
The only "big" things I see are:
- "<obj>.__dict__.get(key, not value) != value" does not work properly for value=None, unless I missed something. Thus you could just as well write "<obj>.__dict__.get(k)", I would think. Same bug, less code ... *shrug*
- Python functions return None by default (if nothing else is explicitly returned), so it isn't necessary to actually write that line. Unless you're into that "clarity" stuff.
1 2 3 4 5 6 7 8 9 10 11
class Planet(object): def __init__(self, all=[]): self.all = all @classmethod def get(self, **kwargs): for p in self.all: match = [False for k,v in kwargs.items() if p.__dict__.get(k) != v] if False not in match: return p
ielectric.myopenid.com
September 28, 2008, September 28, 2008 18:58, permalink
Untested:
1 2 3 4 5 6
class Planet(object): all = [] @classmethod def get(cls, **kwargs): return not any([True for p in cls.all for k,v in kwargs.iteritems() if p.__dict__.get(k) != v])
The get method just doesn't look very pythonic.