On Sun, Aug 9, 2009 at 7:30 PM, Laurens Van Houtven
As you probably know, the interfaces for t.p live in t.p.ipositioning. I still haven't quite understood why they are going to be awesome, but anyway -- I'm running into a problem where keeping them would (I think) imply a bunch of duplicated code.
Have you read this? http://glyph.twistedmatrix.com/2009/02/explaining-why-interfaces-are-great.h...
The thing is, t.p.nmea.NMEAAdapter needs to (at least, I think so) implement all of the methods of t.p.ipositioning.IPositioningProvider, because people will subclass t.p.nmea.NMEAAdapter and override the corresponding (stub) methods.
I don't think this is the case. NMEAAdapter implements INMEAReceiver. It should take an IPositioningProvider as an *argument* somewhere, not implement that interface itself. A user implementing a position-aware application should be encouraged as strongly as possible to avoid any NMEA-specific junk in their application until it comes time to connect the app logic (IPositioningProvider) to the actual device spitting out NMEA data. After all, they're not going to be able to easily make use of cell-tower positioning information if their application class is hard-coded to subclass NMEAAdapter. In order to get all of the stub methods implemented (otherwise people using
the class need to implement *every* method, which is dumb, since a lot of people won't care about things like GPS climb),
The general sentiment here is right, despite the terminology problem I pointed out: somewhere, a user is going to implement an IPositioningProvider and it's quite likely that they won't want to implement every method. So let's assume there's going to be a BasePositioningProvider which stubs out many of these methods or unifies / simplifies them. I would basically be copying every line of the body of the interface into
the t.p.nmea.NMEAAdapter.
Not quite. For one thing, your method declaration lines will have a 'self' in BasePositioningProvider, and in IPositioningProvider they won't. (I notice that this is not currently the case. You should probably fix that: interface method definitions should never include 'self'.) Also, your methods' docstrings won't need to be as complete as those on the interface, since they can just say "implement L{IPositioningProvider.foo} to...". Finally, I hope *most* of the methods will actually have a body that has some implementation; some default behavior that will be useful For example, remembering the last position / heading / speed / error so that 'positionReceived' can conveniently expect the last error received without implementing that method itself. One or two might be completely stubbed out, but the interface can't contain any implementation. This sounds like a dumb idea.
Let's assume the worst case: BasePositioningProvider has absolutely no default behavior besides a no-op. The level of duplication we're talking about here is this: def positionReceived(self, latitude, longitude): def positioningErrorReceived(self, positioningError): def headingReceived(self, heading): def headingErrorReceived(self, headingError): def magneticHeadingReceived(self, magneticHeading): def altitudeReceived(self, altitude): def altitudeErrorReceived(self, altitudeError): def speedReceived(self, speed): def speedErrorReceived(self, speedError): def climbReceived(self, climb): def climbErrorReceived(self, climbError): In other words, the duplicated code in question will be considerably shorter than the body of the message you composed to fret about code duplication, and *way* shorter than my response :-). This doesn't really seem excessive, considering that there's no duplication of *functionality* (the interface only specifies behavior, the base class implements it), only duplication of text. Subclassing the interface sounds like *entirely* the wrong thing to do,
since the class is-not-an Interface.
You're definitely right here. It isn't just conceptually wrong, it's also completely broken. Subclassing an Interface creates another Interface (i.e. instance of InterfaceClass) not a class or type. In other words, you can't instantiate it, and you can't call methods on it if you define any. IIRC, more recent versions of zope.interface will actually emit warnings if you try to put methods which have definitions on an interface. Another alternative is using getattr-with-default-value in order to
call the methods, since then missing (non-implemented) methods will be handled gracefully. I kinda like this one, mainly because it's very little work ;-)
It's more work than you think. If you misspell a method name, then the code which invokes it will fail in a totally straightforward way: it won't invoke your method, and as a result it will probably blow up. If you use getattr-with-default, then it silently does nothing.
A third alternative would be to scrap the interface all together and use a base class with method stubs (I'm deliberately not using the word abstract there). This sounds like a bad idea because we'd be losing the interface, but I'm not entirely sure why we want it in the first place...
I believe the real problem here is that you don't quite get the value of interfaces. Read that blog post that I linked to if you haven't already, and if you have, let's talk about that instead.