Should my classes restrict developers from doing wrong things with them?

Question

I am trying to understand where good contracts end and paranoia starts. Really, I just have no idea what good developer should care about and what shall he leave out :)

Let's say I have a class that holds value(s), like java.lang.Integer. Its instances are aggregated by other objects (MappedObjects), (one-to-many or many-to-many), and often used inside MappedObjects' methods. For performance reasons, I also track these relationships in TreeMap (guava MultiMap, doesn't matter) in addition, to be able to get fast iterations over MappedObjects bound to some range of Integer keys. So, to keep system in consistent state, I should modify MappedObject.bind(Integer integer) method to update my Map like:

class MappedObject {
    public void bind (Integer integer) {
        MegaMap.getInstance().remove(fInteger, this);
        fInteger = integer;
        MegaMap.getInstance().add(fInteger, this);
    }

    ...

    private Integer fInteger;
}

I could just make abstract MappedObject class with this final method, forcing other to inherit from it, but it is rude. If I will define MappedObject as interface with method bind() and provide skeletal implementation -- other developer might later just forget to include it in object and implement method by himself without Map updating.

Superadd

Answer 1

Yes, you should force people to do the right thing with your code. A great example of letting people do the wrong thing is the servlet method init( ServletConfig config ) that expected you would store the servlet config yourself but, obviously, a lot of people forgot to store the config and when running their servlets just failed to work.

When defining APIs, you should always follow the open-closed principle, your class should be open for extension and closed for modification. If your class has to work like this, you should only open extension points where they make sense, all the other functionality should not be available for modification, as it could lead to implementation issues in the future.

Armindaarming

Well, it seems I have no better solution other than final methods in skeletal implementation and capslocked advices to use it in implemented interface's javadoc. – Superadd

Answer 2

Try to focus on functionality first and leave all unnecessary things behind. Btw you can't prohibit reflection so don't worry too much on misuse. On the other hand your API should be clear and straightforward so users will have clear idea, what they should and what they shouldn't do with it.

Tomasz

I strongly disagree with the first sentence! Program carefully and prevent all possibly mistakes (ok, do avoid paranoia). Well, at least with every application that will reach a certain size. Debugging and tracking down odd behaviour is much more time consuming than doing some thinking and some coding beforehand! – Foundation

Of course, users can use reflection to circumvent most security fences. However, having to (ab)use reflections is a high barrier and rarely done by accident. – Leibowitz

Answer 3

At least you should do really everything that prevents bugs but cost no effort.

For example: use primitive types (int) instead of wrappers (Integer) if the variable is not allowed to be null.

So in your bind method. If you not have intended to bind null, then use int instead of Integer as parameter type.

Vassallo

well, Integer in my case is not really an integer :), but potentially a longer-then-Long set of values. I assume I should keep these values in some Comparable wrapper. – Superadd

longer-than-long, I wanted to say – Superadd

Answer 4

I'd say your classes should be designed for as simple use as possible.

If you allow a developer to override methods you definitely should document the contract as good as possible. In that case the developer opts to override some basic functionality and thus is responsible to provide an implementation that adheres to the contract.

In cases where you don't want the developer to override parts of the functionality - for security reasons, if there is no sensible alternative etc. - just make that part final. In your case, the bind method might look like this:

class MappedObject {
public final void bind (Integer integer) {
    MegaMap.getInstance().remove(fInteger);
    internalBind( integer );
    MegaMap.getInstance().add(fInteger);
}

protected void internalBind( Integer integer ) {
 fInteger = integer;
}

...

private Integer fInteger;

}

Here you'd allow the developer to override the internalBind() method but ensure that bind() will do the mapping.

To summarize: Make using and extending classes as easy as (sensibly) possible and don't have the developer to copy lots of boiler plate code (like the map updates in your case) in case he just wants to override some basic functionality (like the actual binding).

Vietnamese

I remember what Bloch wrote, so I would definitely final-ize every method not desinged for extension. The problem is I am not sure client programmer will use my class at all if he has shiny interface to implement. – Superadd

@Superadd you still could provide default implementations for the methods you allow to be overridden. – Vietnamese

Answer 5

If you think your API users are stupid, you should prohibit wrong usage. Otherwise you should not stand in their way to do things they need to do.

Domumentation and good naming of classes and methods should indicate how to use your API.

Aun