Is this a fair question.. or a trap?

[Mike Curwen]

http://www.zhangyining.net/weblog/blog_detail.html?item_id=145

[HS Thomas]

It looks like code you might see in a coding examples book but hope never to see in production.

[Thomas Paul]

Not just that but the question itself is absurd. Are you supposed to be a mind reader? Is he saying that no one should ever be allowed to drop a table? As I mentioned in my comments, what if you are writing a desktop application to support the use of a local database? Why wouldn't you allow someone to drop a table or delete all the rows?

[Yuriy Grechukhin]

My reply to his question "What's wrong with the following Java code?" would be "everything or nothing"
May be you just want to test your little SQL Statement? Aside from not println'ing e.getMessage() this is good function for that.
His statement about 'what if the argument is a string like "DELETE FROM employees" or a DROP TABLE' is also not a very valid point (see above) or validation is done before the function is called (nevermind the function is public, the class might be default).
BUT if it's a production application, and it is explained before the code is presented then all of his points are valid.

[Thomas Paul]

Originally posted by Yuriy Grechukhin:
BUT if it's a production application, and it is explained before the code is presented then all of his points are valid.

What if the purpose of the application is to give a pretty Java front-end to a local MySQL database? Then dropping tables and deleting all the rows might be a perfectly valid thing to do.

[Yuriy Grechukhin]

Originally posted by Thomas Paul:

What if the purpose of the application is to give a pretty Java front-end to a local MySQL database? Then dropping tables and deleting all the rows might be a perfectly valid thing to do.

Then it doesn't follow java naming conventions, as someone noted in the comments of that blog (parameter String sql_str) and it doesn't display Exception messages when you try to close the connection.

[Jim Yingst]

Then it doesn't follow java naming conventions
Why "then"? This point is true, but it has nothing to do with the point that was boing made, and was true regardlss of the purpose of the code.
it doesn't display Exception messages when you try to close the connection
Look again. The only time it fails to display exception messages when closing the connection is if it's already caught some other exception. In this case, I wouldn't really care much about the secondary exception - it's the original exception that I'd want to know about. And this is printed.
I'm not saying this is great code, but it's also not quite as bad as you seem to think.
One problem I haven't seen mentioned is that using this method, you create a new connection for each execution of a SQL statement. This might be fine for a few standalone statements, but could well lead to unncessary performance problems if it's repeated often.

[Yuriy Grechukhin]

Can't... take... the pressure... too many sheriffs attacking...
I think I'm getting arrested!

[Jim Yingst]

[Stefan Wagner]

I'm using java and sql for some time now.
When I saw the code, I wondered, why there is only a boolean return.
If you write every-day user-sql, you mostly have 'select'-statements. But obviously this code isn't meant for select-statements.
So it will be for 'update, insert, delete' I would guess. And why not 'drop table'?
And if the user isn't allowed to 'DROP table', this can (mostly?) and should be managed by his database-account.
Perhaps it's not possible for mySql, which is linked from the website, but serious databases (postgres, informix, oracle) manage permissions by user-accounts.
Jim's excellent description on the exceptions was something, I started to think about, but didn't brought it to an end.
Then I thought I missed the performance - issue with zillions of connections.
But I reread the code, and 'openConnection' is a local method, we don't know how it is implemented. It might return the same connection.
Of course the naming of 'closeConnection' would be a point of criticism, if it doesn't close the connection, but deregisters only the current usage.
But even on standalone-System this may be serious performance issue, leading users to switch to c++, c#, claiming 'java is lame'.
Summa summarum the most important statement is: 'Shall we be mindreader?'.
You may look for syntax-errors, exception handling, deprecated methods, performance, security - some people even look for coding-style! (propagating bad sun-decisions).
I didn't use mySql ever, so can someone tell me, whether there are userpermissions on mySql?
I would say, you may criticise some points of the code, but NOT the security-question of 'dropping tables', even if the method is meant to be called by 'everyone' in a multi-user production system.

[Yuriy Grechukhin]

Originally posted by Stefan Wagner:
You may look for syntax-errors, exception handling, deprecated methods, performance, security - some people even look for coding-style! (propagating bad sun-decisions).

A little off-topic, but i'm just wondering what do you mean by bad sun-decisions.

[Stefan Wagner]

The opening brace in a single line:

if (prettyCode)
{
        foo (bar);
}

is much better readable.