Sonntag, 19. Juni 2011

Why a NotImplementedYetError is needed

Everyone working on non-trivial software projects knows code snippets like the following. Especially if work-in-progress parts (say, a framework) are already used in other parts (like an actual customer project) while still growing and being under construction. Still, there are two things badly wrong with it. What are they?
@Override
public boolean prepend(final E element)
{
 // TODO Auto-generated method stub
 return false;
}

First the positive thing: There's an IDE-processable construct, the TODO, that marks this method as to be implemented for the developer. The IDE can easily list all TODOs, search and filter for particular ones, etc. And the code gets compiled just fine and the class (its already implemented parts) is usable. Fine.
This is completely sufficient if the method has been created just now and gets completely implemented the same day or maybe the next day. That should always be the case in theory and after all is very often the case in practice as well. But the practice also shows that many of those stubs remain. Not for "ever" or for years, of course. At some point, you should really get to implement them and get rid of the stubs. But in all but smaller projects, it can really take up to a few months to fix the last of them (and while you do it, the structure of the still evolving projects still changes a little bit here and there, required new interface methods which in turn to still have compilable classes create... right: more stubs).

So let's just say: until a piece of (non-trivial) software gets to a major release version, it will most certainly contain a number of "not-implemented-yet" stubs. For example the next generation collections framework I'm currently working on contains 224 of them as I type this sentence (which sounds more than it is as many of them will be solved by delegate calls to the same methods, but anyway: I surely won't fix them all today or tomorrow. Some of them maybe not until half a year from now).
Not desirable in theory, but simply reality in practice.

Given this situation, again the question: which two things are badly wrong?
Answer:
The lesser one: there's no annotation (existing in the java language) telling the IDE "this method is not implemented yet, mark it in warnings / intellisense and the like".
But the far greater one: the problem (actually the bug) of not being implemented yet doesn't show at runtime! The method as it stands there (representing all "stubs") is a construction site in disguise, pretending to be a completed, fully functional method. In this case: a boolean returning method called "prepend", taking (and ignoring) one argument of generic type E and always returning false. Why not. Valid code.
One might be tempted to say "well, that's the idea of the error the compiler gives you when you leave out the return". Not quite, because: a) there are void methods, too. And b) the compiler only watches over your syntax, not over the semantics of your code. Meaning it still makes sense to make the syntax compilable even if some parts of the logic it defines are still missing.

The problem is that a stub like this pretends to be something it isn't: finished work. If it gets executed, you have exactely the same problem you had before exceptions and automatic null and array bound checking and type safety etc. were invented: the program just continues, doing stupid things and eventually break, maybe somewhere deep deep inside the program at a completely different point. The solution is always the same: the program has to fail right where the error is, not carry on running around like a zombie that finally stumbles against a tree, but right there.

The first idea to solve this is to replace the vicious return stub code with a "throw new UnsupportedOperationException()". I did that, too, for a long time. At least it causes the program to immediately fail at the right spot. But it's only the half way of what should actually be done. Because it is an unclean, abusive workaround, not a proper means for the situation.
The concept of the UnsupportedOperationException is to cause an exception if a method gets called in an implementation that does not or cannot support the request operation. Can not in terms of "even if it's completely implemented, it can't support this operation". A good example for this are methods like add() or set() in java.util.Collections$UnmodifiableList. The class implements java.util.List which defines add() and set(), but does not and won't ever support them as it's intentionally not modifiable or "immutable" or just a constant list (which, btw. is a terrible misconception in the old JDK collections: If a list implementation is immutable, it should be of a type that only provides reading operations int the first place. Not implement a mindless undifferentiated all-in-one type that defines operations that run against the idea of the implementation - because again you have the situation that some piece of code, the constant list, can pretend to be something else, a general list, be passed along as such a list and only eventualy at some completely different point crash the program. It's a little like having no type system at all).

But that is not the point here. The implementation can and will support the operation, it's just not implemented, yet. So if I keep abusing them for cases like this, all I get is people writing code that eventually calls that method (maybe because the unfinished SubList implementation gets used in a piece of code that normally only deals with the general purpose List implementation), getting an unsupported operation exception and being confused, thinking "it's a normal operation, why isn't it supported?".
The clean, proper, professional way is to have some exception (actually not an exception but a true error) saying "Sorry, this method is not implemented yet, but you can bother the author to move it up in priority right now for you if you need it."

So what is really needed (and sadly one of the many many trivial and basic things that are missing in the JDK) is a "NotImplementedYetError".
The little weird, even funny, thing about that is: It's a piece of code, a part of the software, that should actually never be used in the software (in theory, if all part of a software would be completed immediately). So it's unnecessary by design, but still necessary in practice as a pragmatical "construction site" sign. Maybe it could best be seen as a "meta" error.
Positive side effect: You can easily search the project for all occurances of that class and you'll precisely get all methods and their classes that have construction sites, without getting all the actual occurances of not supported operations that you would have to filter out manually every time.

For these reasons, I finally extended my core project by a "net.jadoth.meta.NotImplementedYetError" class and overhauled all workaround stubs to use them (btw. that's also how I can tell that I currently have 224 of them). Of course I would immediately (and gladly) replace it by a java.meta.NotImplementedYetError() (or a java.lang.NotImplementedYetError or UnimplementedMethodError or whatever, as long as it's the proper means). But I doubt that it will ever happen. Independant of how useful or how needed or how unproblematically orthogonal a new feature is, there are always people screaming "no, this has no business in the java language, it's horribly bad style because of blablabla".

So the example from above now looks like this (and with it, every other stub should look like it from now on). Oh and as a side note: a not yet implemented method is of course an error to be fixed (hence the critical FIXME), not a mere "TODO" like "Maybe, if you have time, come back and implement it".

@Override
public boolean prepend(final E element)
{
 // FIXME Auto-generated method stub
 throw new net.jadoth.meta.NotImplementedYetError();
}

At the end a small note about the mentioned annotation: same problem here, as said. Current workaround is to use @Deprecated, but that is of course no clean solution. Again, an extenstion of Java itself would be required, like @java.lang.NotImplementedYet . I don't bother too much, here, as methods get mostly called via interfaces and not via classes, so the markup would not be visible anyway. The important thing is the runtime error.

Keine Kommentare:

Kommentar posten