Jacob Page wrote: > Thomas Lotze wrote: > > Jacob Page wrote: > > > >>better-named, > > > > Just a quick remark, without even having looked at it yet: the name is not > > really descriptive and runs a chance of misleading people. The example I'm > > thinking of is using zope.interface in the same project: it's customary to > > name interfaces ISomething. > > I've thought of IntervalSet (which is a very long name), IntSet (might > be mistaken for integers), ItvlSet (doesn't roll off the fingers), > ConSet, and many others. Perhaps IntervalSet is the best choice out of > these. I'd love any suggestions.
Hi Jacob, I liked a lot the idea of interval sets; I wonder why no one has thought of it before (hell, why *I* haven't thought about it before ? :)) I haven't had a chance to look into the internals, so my remarks now are only API-related and stylistic: 1. As already noted, ISet is not really descriptive of what the class does. How about RangeSet ? It's not that long and I find it pretty descriptive. In this case, it would be a good idea to change Interval to Range to make the association easier. 2. The module's "helper functions" -- which are usually called factory functions/methods because they are essentially alternative constructors of ISets -- would perhaps better be defined as classmethods of ISet; that's a common way to define instance factories in python. Except for 'eq' and 'ne', the rest define an ISet of a single Interval, so they would rather be classmethods of Interval. Also the function names could take some improvement; at the very least they should not be 2-letter abbreviations. Finally I would omit 'eq', an "interval" of a single value; single values can be given in ISet's constructor anyway. Here's a different layout: class Range(object): # Interval @classmethod def lowerThan(cls, value, closed=False): # lt, for closed==False # le, for closed==True return cls(None, False, value, closed) @classmethod def greaterThan(cls, value, closed=False): # gt, for closed==False # ge, for closed==True return cls(value, closed, None, False) @classmethod def between(cls, min, max, closed=False): # exInterval, for closed==False # incInterval, for closed==True return cls(min, closed, max, closed) class RangeSet(object): # ISet @classmethod def allExcept(cls, value): # ne return cls(Range.lowerThan(value), Range.greaterThan(value)) 3. Having more than one names for the same thing is good to be avoided in general. So keep either "none" or "empty" (preferably "empty" to avoid confusion with None) and remove ISet.__add__ since it is synonym to ISet.__or__. 4. Intervals shouldn't be comparable; the way __cmp__ works is arbitrary and not obvious. 5. Interval.adjacentTo() could take an optional argument to control whether an endpoint is allowed to be in both ranges or not (e.g. whether (1,3], [3, 7] are adjacent or not). 6. Possible ideas in your TODO list: - Implement the whole interface of sets for ISet's so that a client can use either or them transparently. - Add an ISet.remove() for removing elements, Intervals, ISets as complementary to ISet.append(). - More generally, think about mutable vs immutable Intervals and ISets. The sets module in the standard library will give you a good idea of the relevant design and implementation issues. After I look into the module's internals, I'll try to make some changes and send it back to you for feedback. Regards, George -- http://mail.python.org/mailman/listinfo/python-list