If I got promoted every time I spotted code like the one that follows, I'd probably be the President of the United States by now. Take a look at Joe's code here:
public static void main(String[] args) {
String db = args[0];
String uid = args[1];
String pwd = args[2];
String url = "jdbc:postgresql://localhost/" + db;
Connection conn;
try
{
conn = DriverManager.getConnection(url, uid, pwd);
String query = "SELECT A.city, A.zip, st_distance(A.longlat_point_meter, " + "B.longlat_point_meter) FROM utzipcode A, utzipcode B WHERE B.city='MENDON' " + "AND A.city<>'MENDON' AND st_distance(A.longlat_point_meter, B.longlat_point_meter) " + "< 20000 ORDER BY 3;";
Statement stmt = conn.createStatement();
ResultSet rs = stmt.executeQuery(query);
ResultSetMetaData resultMetaData = rs.getMetaData();
System.out.println(resultMetaData.getColumnName(1) + ", " + resultMetaData.getColumnName(2) + ", " +
resultMetaData.getColumnName(3));
while(rs.next()){
System.out.println(rs.getString(1) + ", " + rs.getString(2) + ", " + rs.getString(3));
}
rs.close();
}
catch(SQLException sqle){
System.out.println(sqle.getMessage());
}
}
Can you tell what's going on in the code above? You probably can, with some effort - but why should code so simple be hard to understand? How come the code doesn't tell me what it is doing? Plus, when I'm maintaining this beauty 10 years from now, I'll be hunting Joe down and pulling his fingernails every time I have to debug it - seriously, this code sucks.
Even though just about every programmer I've worked with knows better than to write code that's hard to read, under pressure we all get sloppy. In fact, I was just looking at some code I just wrote a couple of weeks ago as inspiration for this post.
So, now that I've gotten that off my chest, let's see if we can clean this up some. The n00b fix would be to pepper in some comments like this:
public static void main(String[] args) {
String db = args[0];
String uid = args[1];
String pwd = args[2];
String url = "jdbc:postgresql://localhost/" + db;
Connection conn;
try
{
//get the connection to the db
conn = DriverManager.getConnection(url, uid, pwd);
String query = "SELECT A.city, A.zip, st_distance(A.longlat_point_meter, " + "B.longlat_point_meter) FROM utzipcode A, utzipcode B WHERE B.city='MENDON' " + "AND A.city<>'MENDON' AND st_distance(A.longlat_point_meter, B.longlat_point_meter) " + "< 20000 ORDER BY 3;";
Statement stmt = conn.createStatement();
//execute the query defined above
ResultSet rs = stmt.executeQuery(query);
ResultSetMetaData resultMetaData = rs.getMetaData();
//print column names
System.out.println(resultMetaData.getColumnName(1) + ", " + resultMetaData.getColumnName(2) + ", " +
resultMetaData.getColumnName(3));
//print every tuple
while(rs.next()){
System.out.println(rs.getString(1) + ", " + rs.getString(2) + ", " + rs.getString(3));
}
rs.close();
}
catch(SQLException sqle){
System.out.println(sqle.getMessage());
}
}
That still looks terrible, however. Not only that, the comments aren't really that useful; they don't tell me anything I couldn't have figured out by reading the code. In fact, I think these types of comments make the code harder to read since they interrupt what's really important - the code. But that's kind of a side note: the real problem here is that comment's don't give me any insight or explain the intent of the code.
Alright then... What do we do now? Well, how about something like this:
public static void main(String[] args) {
String db = args[0];
String uid = args[1];
String pwd = args[2];
String url = "jdbc:postgresql://localhost/" + db;
Connection conn;
try
{
conn = DriverManager.getConnection(url, uid, pwd);
//we're hard coding the query here because the db is out of SPs - joe
String query = "SELECT A.city, A.zip, st_distance(A.longlat_point_meter, " +
"B.longlat_point_meter) FROM utzipcode A, utzipcode B WHERE B.city='MENDON' " +
"AND A.city<>'MENDON' AND st_distance(A.longlat_point_meter, B.longlat_point_meter) " +
"< 20000 ORDER BY 3;";
ResultSet rs = ExecuteQuery(query, conn);
PrintQueryResults(rs);
rs.close();
}
catch(SQLException sqle){
System.out.println(sqle.getMessage());
}
}
private static ResultSet ExecuteQuery(String query, Connection conn) {
try{
Statement stmt = conn.createStatement();
return stmt.executeQuery(query);
}
catch(SQLException sqle){
return null;
}
}
private static void PrintQueryResults(ResultSet rs){
try{
ResultSetMetaData resultMetaData = rs.getMetaData();
System.out.println(resultMetaData.getColumnName(1) + ", " + resultMetaData.getColumnName(2) + ", " +
resultMetaData.getColumnName(3));
while(rs.next()){
System.out.println(rs.getString(1) + ", " + rs.getString(2) + ", " + rs.getString(3));
}
}
catch(SQLException sqle){ }
}
Ahhh... much better. I can tell what the code is doing in one brief overview - and most importantly, the code itself is telling me what it's doing!
Also, the code is now easier to debug and maintain because each method is only a few lines long; this means I'll know exactly where the code may be having problems next time it breaks by just looking at the stack trace. Finally, look at that comment about the query - that really tells me something I could have never known unless Joe had told me.
So there you go, that's self documenting code in 3 easy steps. Please let Joe know how much you appreciate learning from his mistake by dropping a comment. It'll make him feel better.
2 comments:
Hah, yr my grader.
I think the noob comments are good for the noob. Perhaps he's never coded in Java before, nor has he ever connected to a database via Java. So he wants to be able to look at his own code and find out what's going on.
In the code you describe as very fine, the coder leaves method calls as self-explanatory. They are, they're descriptively named. However, don't each of those methods need the JavaDoc header? Methods need comments, thats just the way it goes.
The point you make about methods is important. Thanks for your tips. :)
@Anonymous:
You're right: the n00b comments are good for the n00b; he needs them in order to understand what he's doing. To some extent it's the same thing small children do: they talk about the action they are performing in order to help them make sense out of them.
I don't think methods always need comments, but doc headers are definitely a good idea for automating automating your documentation.
Thanks for taking the time to comment.
Post a Comment