Subclasses causing unexpected behavior in superclasses — OO design question

Question

Although I'm coding in ObjC, This question is intentionally language-agnostic - it should apply to most OO languages

Let's say I have an "Collection" class, and I want to create a "FilteredCollection" that inherits from "Collection". Filters will be set up at object-creation time, and from them on, the class will behave like a "Collection" with the filters applied to its contents.

I do things the obvious way and subclass Collection. I override all the accessors, and think I've done a pretty neat job - my FilteredCollection looks like it should behave just like a Collection, but with objects that are 'in' it that correspond to my filters being filtered out to users. I think I can happily create FilteredCollections and pass them around my program as Collections.

But I come to testing and - oh no - it's not working. Delving into the debugger, I find that it's because the Collection implementation of some methods is calling the overridden FilteredCollection methods (say, for example, there's a "count" method that Collection relies upon when iterating its objects, but now it's getting the filtered count, because I overrode the count method to give the correct external behaviour).

What's wrong here? Why does it feel like some important principles are being violated despite the fact that it also feels like OO 'should' work this way? What's a general solution to this issue? Is there one?

I know, by the way, that a good 'solution' to this problem in particular would be to filter the objects before I put them into the collection, and not have to change Collection at all, but I'm asking a more general question than that - this is just an example. The more general issue is methods in an opaque superclass that rely on the behaviour of other methods that could be changed by subclasses, and what to do in the case that you want to subclass an object to change behaviour like this.

Answer 1

Your FilteredCollection feels wrong. Usually, when you have a collection and you add a new element into it, you expect that it's count increases by one, and the new element is added to the container.

Your FilteredCollection does not work like this - if you add an item that is filtered, the count of the container might not change. I think this is where your design goes wrong.

If that behaviour is intended, then the contract for count makes it unsuitable for the purpose your member functions are trying to use it for.

Answer 2

The Collection that you inherit from has a certain contract. Users of the class (and that includes the class itself, because it can call its own methods) assume that subclasses obey the contract. If you're lucky, the contract is specified clearly and unambiguously in its documentation...

For example, the contract could say: "if I add an element x, then iterate over the collection, I should get x back". It seems that your FilteredCollection implementation breaks that contract.

There is another problem here: Collection should be an interface, not a concrete implementation. An implementation (e.g. TreeSet) should implement that interface, and of course also obey its contract.

In this case, I think the correct design would be not to inherit from Collection, but rather create FilteredCollection as a "wrapper" around it. Probably FilteredCollection should not implement the Collection interface, because it does not obey the usual contract for collections.

Answer 3

What you described is a quirk of polymorphism. Since you can address an instance of a subclass as an instance of the parent class, you may not know what kind of implementation lies underneath the covers.

I think your solution is pretty simple:

1. You stated that you don't modify the collection, you only apply a filter to it when people fetch from it. Therefore you should not override the count method. All of those elements are in the collection therefore don't lie to the caller.
2. You want the base .count method to behave normally, but you still want the count so you should implement a getFilteredCount method which returns the amount of elements post filtering.

Subclassing is all about the 'Kind of' relationship. What you're doing is not out of the norm but not the most standard use case either. You're applying a filter to a collection, so you can claim that a 'FilteredCollection' is a 'kind of' collection, but in reality you're not actually modifying the collection; you're just wrapping it with a layer that simplifies filtering. In any case, this should work. The only downside is that you have to remember to call 'getFilteredCount' instead of .getCount

Answer 4

I think that the real issue is a misunderstanding of how object-oriented languages are supposed to work. I'm guessing that you have code that looks something like this:

Collection myCollection = myFilteredCollection;

And expect to invoke the methods implemented by the Collection class. Correct?

In a C++ program, this might work, provided that the methods on Collection are not defined as virtual methods. However, this is an artifact of the design goals of C++.

In just about every other object-oriented language, all methods are dispatched dynamically: they use the type of the actual object, not the type of the variable.

If that's not what you're wondering, then read up on the Liskov Substitution Principle, and ask yourself whether you're breaking it. Lots of class hierarchies do.

Answer 5

Rather than sublcassing Collection to implement FilteredCollection, try implememting FilteredCollection as a seporate class that implements iCollection and delegates to an existing collection. This is similar to the Decorator pattern from the Gang of Four.

Partial example:

class FilteredCollection implements ICollection
{
    private ICollection baseCollection;

    public FilteredCollection(ICollection baseCollection)
    {
        this.baseCollection = baseCollection;
    }

    public GetItems()
    {
        return Filter(baseCollection.GetItems());
    }

    private Filter(...)
    {
        //do filter here
    }

}

Implementing FilteredCollection as a decorator for ICollection has the added benefit that you can filter anything that implements ICollection, not just the one class you subclassed.

For added goodness, you can use the Command pattern to inject a specific implementation of Filter() into the FilteredCollection at runtime, eliminating the need to write a different FilteredCollection implementation for every filter you want to apply.

Answer 6

You could argue that the superclass is not well-designed for subclassing, at least not in the way you want to. When the superclass calls "Count()" or "Next()" or whatever, it doesn't have to let that call be overridden. In c++, it can't be overridden unless it's declared "virtual", but that doesn't apply in all languages - for example, Obj-C is inherently virtual if I remember correctly.

It's even worse - this problem can happen to you even if you don't override methods in the superclass - see Subtyping vs Subclassing. See in particular the OOP problems reference in that article.

Answer 7

(Note whilst I'll use your example I'll try to concentrate on the concept rather then tell you what's wrong with your specific example).

Black Box Inheritance?

What you're crashing into is the myth of "Black box inheritance". Its often not actually possible to separate completely implementations that allow inheritance from implementations that use that inheritance. I know this flys in the face of how inheritance is often taught but there it is.

To take your example, its quite reasonable for you to want the consumers of the collection contract to see a Count which matches the number items they can get out of your collection. Its also quite reasonable for code in the inherited base class to access its Count property and get what it expects. Something has to give.

Who is Responsible?

Answer: The base class. To achieve both the goals above the base class needs to handle things differently. Why is this the reponsibility of the base class? Because it allows itself to be inherited from and allowed the member implementation to be overriden. Now it may be in some languages that facilitate an OO design that you aren't given a choice. However that just makes this problem harder to deal with but it still needs be dealt with.

In the example, the base collection class should have its own internal means of determining its actual count in the knowledge that a sub-class may override the existing implementation of Count. Its own implementation of the public and overridable Count property should not impact on the internal operation of the base class but just be a means to acheive the external contract it is implementing.

Of course this means the implementation of the base class isn't as crisp and clean as we would like. That's what I mean by the black box inheritance being a myth, there is some implementation cost just to allow inheritance.

The Bottom Line...

is an inheritable class needs to be coded defensively so that it doesn't rely on assumed operation of overridable members. OR it needs to be very clear in some form of documentation exactly what behaviour is expected from overriden implementations of members (this is common in classes that define abstract members).

Answer 8

The example falls into "Doctor, it hurts when I do this" category. Yes, subclasses can break superclasses in various ways. No, there is no simple waterproof solution to prevent that.

You can seal your superclass (make everything final) if your language supports this but then you lose flexibility. This is the bad kind of defensive programming (the good relies on robust code, the bad relies on strong restrictions).

The best you can do is to act at human level - make sure that the human that writes the subclass understands the superclass. Tutoring/code review, good documentation, unit tests (in roughly this order of importance) can help achieve this. And of course it doesn't hurt to code the base class defensively.

Answer 9

It behaves this way because this is how object-oriented programming is supposed to work!

The whole point of OOP is supposed to be that a sub-class can redefine some of its superclasses methods, and then operations done at the superclass level will get the subclass implementation.

Let's make your example a little more concrete. We create a "Collection animal" that contains dog, cat, lion, and basilisk. Then we create a FilteredCollection domesticAnimal that filters out the lion and basilisk. So now if we iterate over domesticAnimal we expect to see only dog and cat. If we ask for a count of the number of members, would we not expect the result to be "2"? It would surely be curious behavior if we asked the object how many members it had and it said "4", and then when we asked it to list them it only listed 2.

Making the overrides work at the superclass level is an important feature of OOP. It allows us to define a function that takes, in your example, a Collection object as a parameter and operates on it, without knowing or caring whether underneath it is really a "pure" Collection or a FilteredCollection. Everything should work either way. If it's a pure Collection it gets the pure Collection functions; if it's a FilteredCollection it gets the FilteredCollection functions.

If the count is also used internally for other purposes -- like deciding where new elements should go, so that you add what is really a fifth element and it mysteriously overwrites #3 -- then you have a problem in the design of the classes. OOP gives you great power over how classes operate, but with great power comes great responsibility. :-) If a function is used for two different purposes, and you override the implementation to satisfy your requirements for purpose #1, it's up to you to make sure that that doesn't break purpose #2.

Answer 10

My first reaction to your post was the mention of overriding "all the accessors." This is something I've seen a lot of: extending a base class then overriding most of the base class methods. This defeats the purpose of inheritance in my opinion. If you need to override most base class functions then it's time to reconsider why you're extending the class. As said before, an interface may be a better solution, since it loosely couples disparate objects. The sub-class should EXTEND the functionality of the base class, not completely rewrite it. I couldn't help but wonder if you are overriding the base class members then it would seem quite logical that unexpected behavior would occur.

Answer 11

When I first grok'd how inheritance worked I used it a lot. I had these big trees with everything connected one way or another.

What a pain.

For what you want, you should be referencing your object, not extending it.

Also, I'd personally hide any trace of passing a collection from my public API (and, in general, my private API as well). Collections are impossible to make safe. Wrapping a collection (Come on, what's it used for??? You can guess just from the signature, right?) inside a WordCount class or a UsersWithAges class or a AnimalsAndFootCount class can make a lot more sense.

Also having methods like wordCount.getMostUsedWord(), usersWithAges.getUsersOverEighteen() and animalsAndFootCount.getBipeds() method moves repetitive utility functionality scattered throughout your code into your new-fangled business collection where it belongs.