Look at some of DBell's code:
http://www.tip.net.au/~dbell/
http://www.isthe.com/chongo/tech/comp/calc/index.html
Learn from it. DBell is one of the best programmers on the planet.
I have nothing against Mr DBell, whom I don't know, but... don't learn from it, please. Weather 1.9 - Java application to plot weather observations from the Australian Government's Bureau of Meteorology (BOM) web site.
I did look at it.Controller.java is over 1200 lines long. Don't learn from people who create classes that big (aka God objects).
SunriseSunset.java contains nearly 100 lines of commented out code. That's another bad practice.
What's more, it consists mostly of a main method commented as "Simple main to test the class". This is a so-called poor man's unit test, it doesn't use assertions, it just prints some results out to be verified by the programmer manually. Which is another antipattern.
His FileInfo.java (part of his FileSelection 2.0) contains tautological comments like:
//
// Get the suffix path.
//
String suffixPath = fullPath.substring(prefixPath.length());
Gee, you don't say. Redundant comments, another antipattern.isDirectoryEmpty (in the same file) happily ignores an exception:
boolean isEmpty = true;
try
{
stream = Files.newDirectoryStream(directoryPath);
for (Path path : stream)
{
String name = path.getFileName().toString();
if (!name.equals(".") && !name.equals(".."))
{
isEmpty = false;
break;
}
}
stream.close();
stream = null;
}
catch (IOException e)
{
}
finally
{
Utils.safeClose(stream);
}
return isEmpty;
Meaning that if an IOException occurs (for whatever reason), the method will tell you that the directory is empty, even if that's not true.Setting stream to null after closing it makes no sense whatsoever (it's a local variable, it's not going to survive exiting the method).
Closing it in the try block makes no sense to me either, because Utils.safeClose is going to execute either way; that's how the finally clause works. It looks like the author assumed that finally only executes if an exception happens. No, it is run in either case.
So, what's the point of closing the stream twice? And if it does serve some purpose, now that would require leaving a comment, because it's totally unobvious. Not "getting the suffix path", which is blatantly obvious.
I could go on. This is poor quality code, I'm hardly a great programmer (working on it!), but it's not up to my production standards.
If the man is "one of the best programmers on the planet", the planet can't be Earth.