Saturday, July 14, 2007

The need for RAE

With all the discussions around language changes in Java7, there is a need for democratic vote on what should be included or not.
But in today's Bug Database of Sun you can only vote for a RFE (Request For Enhancements), you cannot vote against. And when you look inside the Top 25 RFEs you can see that there is big controversy on most of them, and that Sun act as a final "benevolent dictator".
For example here is an extract of the top 25 RFEs that may have an impact on Java7/OpenJDK and the language:
Votes Bug ID Synopsis
580 4449383 Support For 'Design by Contract', beyond "a simple assertion facility"
341 4820062 Provide "struct" syntax in the Java language
303 4267080 break up rt.jar into downloadable-on-demand components to reduce jre size
197 4093687 Extension of 'Interface' definition to include class (static) methods.
172 4905919 RFE: Operator overloading
171 4801527 Support Repository in Java Web Start
152 4727550 Advanced & Raw Socket Support (ICMP, ICMPv6, ping, traceroute, ...)
125 4313887 New I/O: Improved filesystem interface
122 4129445 An API to incrementally update ZIP files
122 4650689 RFE: Java needs public API for FTP
113 4648386 Simplify deployment and versioning by embedding JAR files within each other

From this list, I'm personally for "java kernel: 4267080", "New IO: 4313887", against "Operator overloading: 4905919", "Design By Contract: 4449383", and still undecided on "struct: 4820062".
So, how can we compile all the votes?
Sun needs to create RAE (Request Against Enhancements) in parallel of each controversial RFE.
To compile the votes, we will need better access to the Bug Database, since listing all RFEs and RAEs by importance is impossible from the current Web Interface.
The need for RAE came from the big controversy around checked exceptions and I was really missing a nice compilation result of for/against.

Today, Java is Open Source and the full activation of the OpenJDK Governance Board is in progress. I will really like to know what to expect in the language. Having a compilation of popular RFE/RAE will help.
But until the process of the Governance Board is not clarified, Sun position will be the decisive one. And here the current position of Danny Coward, JavaSE Platform lead, is very confusing: "Seeking a small number of changes for Java SE 7 platform" (from Java One presentation).

Tuesday, July 10, 2007

Voting for Good Exception Handling

When getting ready for a conference I gave for Sun (Java 7 - A lot to be waiting for !), I came across this blog: Voting for Checked Exceptions. It was in response of Neal Gafter's blog: Removing Language Features?.
The list of comments is very long, and I never found the time to read them, until now. I really liked what went on there (except the personal attacks), and I want to try to summarize the arguments, as I view them. The arguments are also exposed in Neal's blog comments but there was a lot less arguments...
I'm not trying to be impartial, since for me it's clear: Checked Exceptions should go.
Page 49 of the presentation has the bullets that summarize my view on Checked Exceptions.

So here are the valid arguments I saw in the blog comments:

Checked Exceptions avoid the digging of what can go wrong

Description:

In other languages that does not have checked exceptions, it takes a good amount of time, thinking, QA cycles, and production crashes before you know which exception types can be thrown by a specific method.
Basically, since the API does not declare what it can throws, as a caller you need to "guess" what can happen or catch everything and try to re-throw what should not have been catch.

Validity:

The point is totally valid, but for me it does not make the balance tip on either side of the checked/unchecked issue.
Good API should declare a good throws clause list of what can happen. This is good design and OO practice. And a caller that knows what to do in case of an exception should catch them and handle them correctly. This is valid for Exceptions in general, checked or unchecked. The issue is that in Java nobody declares unchecked exceptions in the throws clause, it looks stupid. I don't know why, I think it's just a stupid habit.

Still there is a point against checked exceptions here. As an API writer, you are "forcing" the caller to catch ALL of the "checked" exceptions declared in your throws clause. This simple fact, breaks OO, forces some design concept on the caller (where to put the exception handler and method throws clauses), and generate all the "bloat".


Checked Exceptions helps you remember that you need to handle them

Description:
During coding you encounter methods throwing exceptions and so you need to handle them at some point. So, with checked exceptions, for a certain class or module, the list of exceptions to handle is well define.
This point was made especially when some lazy developers will ignore unchecked exceptions all together. If the compiler will not bother, exceptions are ignore.

Validity:
Here the point fails on multiple accounts:
1) Forcing the catching of an exception on a lazy developer is worse than let him go along. You end up with things like:
try {
o = getObject(id);
}
catch (PersistenceException e) {
// Should not happen I just saved it before
}
Which is my personal nightmare on many projects. When the code is full of these it's totally impossible to debug, understand the behavior, or move forward. It's desperate. Simply put:
"Empty catch block are worse than no catch at all"

2) Listing and catching all checked exceptions does not cover your needs in exception handling. Even in Java there are unchecked exceptions, and in today's framework you have more and more of them. So exceptions will slip through anyway.

3) Putting the error handling in the middle of the logical code is a very bad practice. You end up copy/pasting catch block all over, and if you need a small change in error handling, well... forget it.
The error handling is a layer of your module/component/architecture, that should be transparent and decoupled from your code.
This is the only way to make sure you are handling correctly ALL exceptions. This is the power and design in modern framework using AOP Interceptors. So, like in Spring, JBossMC, EJB3, Hibernate, you need a framework layer to catch Throwable and find out which exception handler should take care of what at which layer. Perfect, unbeatable, robust, user friendly, HA, and more.


You don't have to handle checked exception just pass them on

Description:
If inside a method you are calling a method with checked exceptions and you don't know how to handle it, just add it to your (big) list of throws clause, someone up the call stack will take care of it.

Validity:
This is the most non valid argument for checked exceptions, and the main reason why Java should have only unchecked exceptions.
I don't know many projects where any developer can just add a checked exception to the signature without blinking (the same for removing one). Checked Exceptions makes API throws clause fixed for eternity. If you add, you suffer, if you remove, you're lynched...
So, the only way is to create non meaningful generic Super Exception class (like IOException) that every methods throws.
Basically, this argument contradict the 2 previous ones which are a lot more valid.
On the other hand this feature is mandatory for normal coding. Everybody agree:
"If you don't know what to do with an exception, don't handle it"

This is critical, for "robust" application, so you need to find a way to answer this argument. Here is the proof that checked exceptions are contradicting their own benefits.


Checked Exceptions are recoverable, Unchecked are bugs

Description:
The principle of separation between checked/unchecked is well defined by ELH:
"as in section 8 of Effective Java. [...] checked exceptions are for unpredictable environmental conditions such as I/O errors and and XML well-formedness violations while unchecked exceptions are for program failures that should be caught during testing, such as array index out of bounds or null pointers."

Validity:
Classifying, and understanding what the error IS, is a critical step in good error handling. You need to send a good type of exception, for the good type of error, with the maximum amount of information (which file, to read, to write, which DB, which XML tag, ...).
Forcing yourself to create meaningful Exception is a difficult step, but one that always pays off. Add all the parameters you can, create meaningful messages, and use the good exception class.
But, the separation "unpredictable environmental conditions" vs. "program failures" is highly subjective, and always false once you climb one layer in your architecture. The kind of comment "Should not happen I just saved it before" proves my point.
Typing is good (IOException, NPE), but to enforce the global decision for all Java software ever written that IOException is an "unpredictable environmental conditions" does not make any sense.
Furthermore, the "forcing" of handling of "unpredictable environmental conditions" generates a lot of "development by exceptions". Basically, if you know that the file may not be there, I really prefer if the developer just create a File() object and test file.exists() instead of waiting for IOException.

Experiences with Checked Exceptions

1) In my experience to handle correctly an exception you need a lot of infrastructure information: How to display errors to the user (Web or Fat client), How to log, How to translate (code, language), How to trap, Severity detector, transaction access, ...
And to provide all this information to the inner logical code that need to wrap checked exceptions is a big burden for the application. Just pass the exception, my exception handler will handle it correctly.
Since, creating an exception handler with IOC and/or AOP that has access to all the above managers, is quite easy and clean.

2) We are doing lately a lot of migration to JPA, and more than 50% of our headaches are on checked exceptions. We found out that checked exceptions promotes indirectly the bad habit of development by exceptions:
try {
o = getObject(
id);
}
catch (PersistenceException e) {
// Object not found create it
o = create();
}
And changing the API signatures, removing FinderException from EJB 2.0, or the annoying CloneableException is a true nightmare.

3) Good coding with checked exceptions is sometimes very very tedious. For example the correct management of FileInputStream.close() and it's IOException is getting out of hand:
FileInputStream is = null;
try {
is =
new FileInputStream(fileName);

// Some work...
int n = is.read();

// Need to close here so caller will know file in unstable state
is.close();
// Need to set null to avoid double close
is = null;
}
catch (IOException e) {
throw new PersistenceException("Error reading file "+fileName, e);
}
finally {
if (is != null) {
try {
is.close();
}
catch (IOException ignore) {
// Well I try to close, but it does not close
// SO may be I should close ;-)
log.debug("Ignoring on close of ", ignore);
}
}
}

Conclusion
Independently, of the discussion on closures, I really hope that the compiler enforcement of catching checked exceptions will be removed in the openJDK very soon...

Saturday, June 23, 2007

PATCH Abstract Enum with OpenJDK

The full solution in OpenJDK

Applying from KSL to OpenJDK

First, there are no issues (except a diff of 2 lines in the GPL license block ;-) to apply a patch from KSL to the OpenJDK project trunk checkout.
So I applied my KSL patch and I started modifying "sun/reflect/annotation/AnnotationParser.java" to retrieve the Enum.valueOf() from the parameter class type and not from the type definition. It means that in:
@AnnoAETest(ae2 = E21.one)
private String first;
where:
public @interface AnnoAETest {
AbstractE2 ae2();
}
the AnnotationParser was doing Enum.valueOf(AbstractE2.class,"one"), but it had actually the class E21 defined, so I changed it to the good Enum.valueOf(E21.class,"one").
So the patch is:
 Index: j2se/src/share/classes/sun/reflect/annotation/AnnotationParser.java
===================================================================
--- j2se/src/share/classes/sun/reflect/annotation/AnnotationParser.java (revision 237)
+++ j2se/src/share/classes/sun/reflect/annotation/AnnotationParser.java (working copy)
@@ -422,9 +422,12 @@
if (!enumType.getName().equals(typeName))
return new AnnotationTypeMismatchExceptionProxy(
typeName + "." + constName);
- } else if (enumType != parseSig(typeName, container)) {
- return new AnnotationTypeMismatchExceptionProxy(
- typeName + "." + constName);
+ } else {
+ Class paramEnumType = parseSig(typeName, container);
+ if (!enumType.isAssignableFrom(paramEnumType))
+ return new AnnotationTypeMismatchExceptionProxy(
+ typeName + "." + constName);
+ enumType = paramEnumType;
}

try {
Now, another issue appears, is that the complicated test to know if a class is an enum with member or an abstract enum or anonymous class is complicated and the Class.isEnum() implementation was wrong.
After changing to the following implementation to remove specialized enum (anonymous):
 Index: j2se/src/share/classes/java/lang/Class.java
===================================================================
--- j2se/src/share/classes/java/lang/Class.java (revision 237)
+++ j2se/src/share/classes/java/lang/Class.java (working copy)
@@ -2877,8 +2877,7 @@
// An enum must both directly extend java.lang.Enum and have
// the ENUM bit set; classes for specialized enum constants
// don't do the former.
- return (this.getModifiers() & ENUM) != 0 &&
- this.getSuperclass() == java.lang.Enum.class;
+ return ((this.getModifiers() & ENUM) != 0 && !isAnonymousClass());
}

// Fetches the factory for reflective objects
I'm getting for my following last failed test:
public class AnnoAEUsage {
@AnnoAETest(ae2 = E21.one)
private String first;

public static void main(String[] args) throws Exception {
System.out.println("Is enum "+E21.class.isEnum());
E21 e21 = Enum.valueOf(E21.class, "one");
switch (e21) {
case one:
System.out.println("Got 1");
break;
default:
System.err.println("No got 1");
}
Field field = AnnoAEUsage.class.getDeclaredField("first");
AnnoAETest anno = field.getAnnotation(AnnoAETest.class);
AbstractE2 ae2 = anno.ae2();
System.out.println("Youpi again "+ae2.full());
}
}

enum E21 extends AbstractE2 {
one, two;
}

The following output:

JDK Build:/work/java.net/ksl.trunk$/work/java.net/openjdk.trunk/control/build/linux-amd64/j2sdk-image/bin/java -cp build/test-classes abstractEnum.annotation.AnnoAEUsage
Is enum true
Got 1
Exception in thread "main" java.lang.IllegalAccessError: tried to access class abstractEnum.annotation.AbstractE2 from class $Proxy3
at $Proxy3.ae2(Unknown Source)
at abstractEnum.annotation.AnnoAEUsage.main(AnnoAEUsage.java:32)

Some more work...

Stupid isn't it
Well it turn out this one is logical...
It's related to an open bug: 6256803 which is not really a bug in my view. You just need to expose public your interfaces, since the dynamic proxy implementing annotation interface is not part of the package. So my AbstractE2 abstract enum, and the E22, E21 real enums were package protected, so: IllegalAccessError.
After making all the test classes public, it works...

The final OpenJDK Patch
For the b14 of openJDK I needed to apply for my platform: 6567018
The above patch file is for "hotspot/src/share/vm/gc_implementation/"

The root project for my patches (KSL and OpenJDK) and all tests files is here.

For the OpenJDK all patches were executed under:
URL: https://openjdk.dev.java.net/svn/openjdk/jdk/trunk
Repository Root: https://openjdk.dev.java.net/svn/openjdk
Revision: 239
And they can be downloaded here.

What's next
So, now what's left to do is to remove the "extends Enum" and make the generics declarations ">" generated by synthetic sugar.
I think that during the work on generics I will try to see the difficulty of having generics for Enum also?
But before all that I think I will start to apply this internal JDK to the fields-enum and other projects, to really judge the improvements...

The "reopened" RFE was accepted
Thank you for taking the time to suggest this enhancement to the Java Standard Edition.

We have determined that this report is an RFE and has been entered into our internal RFE tracking system under Bug Id: 6570766

1. Voting for the RFE
Click http://bugs.sun.com/bugdatabase/addVote.do?bug_id=6570766

2. Adding the report to your Bug Watch list.
You will receive an email notification when this RFE is updated. Click http://bugs.sun.com/bugdatabase/addBugWatch.do?bug_id=6570766