I'd recommend that you read PEP 8, which contains the community style guide for Python code. Pythonistas get very picky about code formatting and style conventions. In particular, see Pet Peeves, which recommends against writing:
def len( self ):
return len( self.theElements )
in favor of:
def len(self):
return len(self.theElements)
without the extra whitespace inside the parentheses.
Of course, a foolish consistency is the hobgoblin of small minds, but it's good to be aware of PEP 8 so that when you go against it, you do so knowingly and for a good reason.
The community is sort of lukewarm on using an initial underscore to indicate a "private" member see https://stackoverflow.com/questions/551038/private-implementation-class-in-python and note how ambivalent everyone seems about the convention. After spending some time with Clojure, I never use any kind of "private" modifier in Python, but I'm sort of an extremist in that regard, and it can be useful if you're writing code that will be used by lots of other people.
On to more specific things. The list constructor can take another iterable as a parameter, and will automatically add all the elements of that iterable to the newly created list. So you could write your __init__ method like this:
def __init__(self, *args):
self._theElements = list(args)
It seems unnecessary to use an assertion to have remove require that items to be removed are actually in the set. Assertions are typically more for checking that things which should never happen ever haven't happened; they essentially say "This condition is so disastrous that if it does happen, the entire system should just die", and as such, they're usually more useful in development, where you want the entire system to die so you can debug your code.
Your code using the set might not ever need to care that it tried to remove something which hadn't been added, in which case you could write something like this:
def remove(self, element):
try:
self._theElements.remove(element)
except ValueError:
pass
Or there might be some cases where you need to care. Then I'd recommend something like this:
def remove(self, element):
try:
self._theElements.remove(element)
except ValueError:
raise ValueError("The element {0} is not in the set".format(element))
Or like this, with return codes:
def remove(self, element):
try:
self._theElements.remove(element)
except ValueError:
return None
else:
# Executed when no exceptions raised.
return self
The main argument in favor of the second version is that with this, users of the set can sometimes care that the item they tried to delete wasn't in the set, and other times can ignore it. The first version forces them to always care. Also note that I threw a new ValueError in the first version so I could add a more descriptive message.
I favor one of these three options because I can't foresee many circumstances where the set not containing the element you tried to delete is so disastrous that the entire system should just die. But if that is what you want, you can still have the client code kill the entire system on a ValueError or on a return value of None.
If you're interested in extending this class, there are a few tricks you could play around with that might help performance in some cases. One would be to keep your list of elements sorted, and use a binary search to check if the set contains a particular element. You could have add sort the list, and then have __contains__ call out to a binary search function. This will also make searching for duplicates faster.
For the most part, you've done a good job implementing a set ADT and have exercised a lot of the neat OO features of Python, like overriding __contains__. The issues I brought up are pretty minor in all. If you ever really need a set in Python, though, I recommend using the built-in set; it's backed by a C hash table and is amazingly fast, as Python code goes.
- 1k
- 5
- 18