Hi, Chris,
code cleanups are always welcome, but should be done with care. Even
small refactorings could result in issues in someone else' apps.
Some minor comments below.
On 10/20/16, 2:32 PM, Chris wrote:
While working on SkinJob, I ran IntelliJ's Code Inspector tool over
OpenJDK AWT, and I found lots of room for improvement, much of which
could be automated. Unused imports and weird indentation are the order
Wrong indentations make code less readable, but they are rarely fixed as
a part of bug fixes to avoid file history pollution. People often need
to look at all changesets of a file to find out when certain lines were
changed and what for. If a bug fix is just 5 lines of code, but
additionally touches 80 lines just to address indentation, it can be
very hard to understand what the fix was really about.
of the day. Raw types are often used in place of generics, leading to a
lot of unnecessary explicit casts. The deprecated Vector and Hashtable
Raw types should be handled with care as well, as they may be a part of
method/class signature and cannot be changed because of backwards
compatibility.
are used in a number of places where I'm pretty sure ArrayList<> and
HashMap<> would work fine and perform better. Many abstract classes have
only one implementation. Switch statements are often missing a default
Are these classes public/protected? Some private or package private
classes used to be extended in the past, and in this case they are good
candidates for refactoring.
case, which means methods will fail silently if a parameter value comes
from the wrong pseudo-enum. When they do have a default case, it seems
to be at the top as often as the bottom. Magic numbers that I can easily
see a developer wanting to change (e.g. the default font size of 12) are
copied in many different places. Even when named constants for things
like color-channel bit masks exist, they aren't consistently used. There
These are all good candidates for cleanup.
also seem to be a fair number of unused methods that aren't public or
are buried in sun.* (and thus should be safe to delete), and a few
inconsistencies involving equals and hashCode.
Unused methods that are safe to delete - should be very careful about
this. Many methods and classes are used from the native code, which is
not tracked by IntelliJ code inspector (I believe).
equals() and hashCode() inconsistencies should indeed be fixed.
Is there a project underway to clean up these sorts of issues? If so,
I'd like to help.
I don't think such a project (in terms of OpenJDK projects) exists. Such
issues are addressed one by one, usually grouped by issue type (e.g. fix
equals/hashCode, fix default cases in switches, fix javadoc links, etc.)
by each OpenJDK group.
Thanks,
Artem
