patternpythonMinor
Python simple int subclass
Viewed 0 times
subclasssimplepythonint
Problem
The below is a class designed to implement Rememember the Milk's task priorities. I'm using it partly as an exercise in "Pythonic" programming, so it's fairly simple already, but advice on how to make it simpler or more Pythonic would be particularly appreciated.
I'm using Python 2.6.5 in case it makes any difference.
Some selected output, for the curious:
I'm using Python 2.6.5 in case it makes any difference.
class Pri(int): # Subclass int to get handy comparison functions etc.
'''Task priority'''
public_from_internal = {1: 1, 2: 2, 3: 3, 4: 'N'}
internal_from_public_str = {'N': 4, '1': 1, '2': 2, '3': 3}
_dict = dict()
def __new__(cls, level):
try:
level = Pri.internal_from_public_str[str(level)]
except KeyError:
raise ValueError, "Pri must be 'N', '1', '2', or '3'"
# If there's already an instance of this priority, don't create a new one.
if level in Pri._dict: return Pri._dict[level]
else: return super(Pri, cls).__new__(cls, level)
def __init__(self, level):
super(Pri, self).__init__()
Pri._dict[self] = self
def __repr__(self): return "Pri('" + str(Pri.public_from_internal[self]) + "')"
# Priority 1 is clearly greater than priority 3, so invert cmp
def __cmp__(self, other): return (-cmp(int(self), int(other)))Some selected output, for the curious:
>>> Pri(1)
Pri('1')
>>> Pri('N')
Pri('N')
>>> Pri(4)
Traceback (most recent call last):
File "", line 1, in
File "rtm.py", line 10, in __new__
raise ValueError, "Pri must be 'N', '1', '2', or '3'"
ValueError: Pri must be 'N', '1', '2', or '3'
>>> Pri._dict
{Pri('1'): Pri('1'), Pri('N'): Pri('N')}
>>> Pri(1) > Pri(2)
True
>>> bool(Pri('N'))
True
>>> repr(Pri("2"))
"Pri('2')"
>>> print repr(Pri('N'))
Pri('N')
>>> Pri(1)._dict
{Pri('1'): Pri('1'), Pri('2'): Pri('2'), Pri('N'): Pri('N')}Solution
class Pri(int): # Subclass int to get handy comparison functions etc.Abbreviating words in class names makes it harder to follow. Just call it
Priority'''Task priority'''
public_from_internal = {1: 1, 2: 2, 3: 3, 4: 'N'}
internal_from_public_str = {'N': 4, '1': 1, '2': 2, '3': 3}
_dict = dict()Naming this
_dict doesn't give me a hint about what its used fordef __new__(cls, level):
try:
level = Pri.internal_from_public_str[str(level)]I don't like stringifying anything that gets passed in here. You really only want to support strings and ints not things that happen to stringify into the correct types.
except KeyError:
raise ValueError, "Pri must be 'N', '1', '2', or '3'"
# If there's already an instance of this priority, don't create a new one.
if level in Pri._dict: return Pri._dict[level]
else: return super(Pri, cls).__new__(cls, level)Use the following construct instead:
try:
return Pri._dict[level]
except KeyError:
return super(Pri,cls).__new__(cls, level)I think its clearer, and it will be slightly faster.
def __init__(self, level):
super(Pri, self).__init__()
Pri._dict[self] = selfI do this is
__new__ and not have an __init__ function.def __repr__(self): return "Pri('" + str(Pri.public_from_internal[self]) + "')"I don't like putting the method name and implementation on the same line. I think it makes it look clutered. I'd also recommend using
return "Pri(%s)" % Pri.public_from_internal[self] as I think its clearer as to the result. # Priority 1 is clearly greater than priority 3, so invert cmp
def __cmp__(self, other): return (-cmp(int(self), int(other)))You inherited from int to get comparisons. But then you have to reimplement the comparisons anyways. Now you also have all the other stuff like adding/multipyling/subtracting/etc which don't make sense to apply to priorities. Basically, there is no reason to inherit from int here, and many reasons not to.
How I'd do this
class Priority(object):
priorities_by_name = {}
@classmethod
def from_string(cls, name):
return cls.priorities_by_name[name]
priorities_by_number = {}
@classmethod
def from_number(cls, number):
return cls.priorities_by_number[number]
def __init__(self, name, number):
self.priorities_by_name[name] = self
self.priorities_by_number[number] = self
self.name = name
self.number = number
def __repr__(self):
return "Priority" % self.number
def __cmp__(self, other):
return -cmp(self.number, other.number)
Priority.First = Priority('1', 1)
Priority.Second = Priority('2', 2)
Priority.Third = Priority('3', 3)
Priority.None = Priority('N', 4)
Priority.__init__ = None # Prevent any future creations- I explicitly call class method to get a priority from either the string or number. I dislike overloading the one constructor to take either.
- For a small number of items, I prefer to create them all upfront. It makes the construction simpler
- I can access Priority objects at Priority.First (etc)
Truth be told I probably wouldn't implement a Priority class, instead I'd keep track of the priority as an int.
Code Snippets
class Pri(int): # Subclass int to get handy comparison functions etc.'''Task priority'''
public_from_internal = {1: 1, 2: 2, 3: 3, 4: 'N'}
internal_from_public_str = {'N': 4, '1': 1, '2': 2, '3': 3}
_dict = dict()def __new__(cls, level):
try:
level = Pri.internal_from_public_str[str(level)]except KeyError:
raise ValueError, "Pri must be 'N', '1', '2', or '3'"
# If there's already an instance of this priority, don't create a new one.
if level in Pri._dict: return Pri._dict[level]
else: return super(Pri, cls).__new__(cls, level)try:
return Pri._dict[level]
except KeyError:
return super(Pri,cls).__new__(cls, level)Context
StackExchange Code Review Q#6229, answer score: 3
Revisions (0)
No revisions yet.