Monday, August 22, 2011

Lesson for the day: Check your assumptions!

So I mentioned earlier that I had some concerns about the interrelationship of some of the classes I'd defined - enough so that I felt I needed to take some time and draw up a UML-ish diagram of them.

I don't know where I got the assumption that there was a problem, in retrospect. The concern I had was that the BaseFilesystemItemCache and IFilesystemItem classes (respectively, a nominal abstract class and a nominal interface) both needed to know about each other in order for a filesystem item to be deleted both as the object-instance representing the actual item, and within the item-cache that is being provided. That would require that each IFilesystemItem-derived instance needing to know what a BaseFilesystemItemCache is (in order to pass one to the constructor and be able to type-check it at construction), and be able to pass a "remove" call on to it's cache. At the same time, any BaseFilesystemItemCache instance would need to know what an IFilesystemItem is, in order to type-check that items being added conform to interface expectations.

Simplified, the relevant functionality would look something like this:

class BaseFilesystemItemCache( object ):
    def __init__( self ):
        if self.__class__ == BaseFilesystemItemCache:
            raise NotImplementedError( 'BaseFilesystemItemCache is nominally an abstract class, and should not be instantiated.' )
        self.Cache = []
    def Add( self, item ):
        if not isinstance( item, IFilesystemItem ):
            raise TypeError( '%s.Add error: Expected an instance implementing IFilesystemItem.' % ( self.__class__.__name__ ) )
        self.Cache.append( item )
        item.Cache = self
    def Remove( self, item ):
        if item in self.Cache:
            self.Cache.remove( item )

class IFilesystemItem( object ):
    def __init__( self, cache=None ):
        self.Cache = None
        if self.__class__ == IFilesystemItem:
            raise NotImplementedError( 'IFilesystemItem is nominally an interface, and should not be instantiated.' )
        if cache != None and not isinstance( cache, BaseFilesystemItemCache ):
            raise TypeError( '%s error: Expected a BaseFilesystemItemCache instance for cache' % ( self.__class__.__name__ ) )
        if cache:
            self.Cache = cache
    def __del__( self ):
        if self.Cache:
            self.Cache.Remove( self )

class TestCache( BaseFilesystemItemCache ):
    def __init__( self ):
        BaseFilesystemItemCache.__init__( self )

class TestItem( IFilesystemItem ):
    def __init__( self, cache=None ):
        IFilesystemItem.__init__( self, cache )

Given what I thought I knew, my expectation was that an error would be raised somewhere along the line, just because of the interpretation/compilation sequence that I thought Python used.

But this code runs (it doesn't do anything, but it raises no errors, either).

I added a few more lines to create a TestCache and a couple of TestItems:

cache = TestCache()
item1 = TestItem()
item2 = TestItem()

# At this point, nothing's been added to the cache:
print "Empty Cache"
print cache.Cache
print

print "Caching item2", item2
cache.Add( item2 )
print cache.Cache
print

print "Caching item1", item1
cache.Add( item1 )
print cache.Cache
print

print "Removing item2"
cache.Remove( item2 )
print cache.Cache

and it shows that the cache add/remove cycle works as I'd originally wanted:

Empty Cache
[]

Caching item2 __main__.TestItem object at 0xb769f2ac
[__main__.TestItem object at 0xb769f2ac]

Caching item1 __main__.TestItem object at 0xb769f28c
[__main__.TestItem object at 0xb769f2ac, __main__.TestItem object at 0xb769f28c]

Removing item2
[__main__.TestItem object at 0xb769f28c]

In retrospect, I think that I was thinking of object inheritance restrictions - a class cannot derive from another class that isn't earlier in the same file or external - though I'm really not certain how I came to the assumption I did.

So: Check your assumptions... Lesson learned...

No comments:

Post a Comment