debugjavaspringMinor
Handling an invalid SQL query exception
Viewed 0 times
handlingexceptionsqlqueryinvalid
Problem
This is my DAO object that takes the Cypher query as a string, and returns a JSON representation of the graph it returns.
Because it's using a Spring
What I'm wanting to do here, is detect whether the error being thrown is due to an invalid query (in which case I want to return a message to the user saying so), or for now, any other error, in which case I'll file it as 'An unexpected error occurred'.
When I call the DAO from my Business Layer:
Is this an OK way of doing this?
One question I have is that both
Because it's using a Spring
JdbcTemplate to access the database, the jdbc query will return any errors wrapped in a DataAccessException. What I'm wanting to do here, is detect whether the error being thrown is due to an invalid query (in which case I want to return a message to the user saying so), or for now, any other error, in which case I'll file it as 'An unexpected error occurred'.
public String runQuery(String q) throws JsonGenerationException, JsonMappingException, UncategorizedSQLException, DataAccessException
{
Graph g = null;
try {
g = jdbcTemplate.query(q, new Neo4jGraphResultSetExtractor());
} catch (DataAccessException e) {
if (e instanceof UncategorizedSQLException)
{
//Invalid cypher query
throw (UncategorizedSQLException)e;
}
else throw e;
}
json = g.toJson(); //throws JsonGenerationException, JsonMappingException
return json;
}When I call the DAO from my Business Layer:
public String submitCypherQuery(String query) {
try {
return neoConn.runQuery(query);
} catch (ClassNotFoundException e) {
return handleUnexpectedError(e);
} catch (JsonGenerationException e) {
return handleUnexpectedError(e);
} catch (JsonMappingException e) {
return handleUnexpectedError(e);
} catch (IOException e) {
return handleUnexpectedError(e);
}
catch (UncategorizedSQLException e)
{
return new ErrorMessage(errorInvalidQuery).toJson();
}
catch (DataAccessException e)
{
return handleUnexpectedError(e);
}
}Is this an OK way of doing this?
One question I have is that both
UncategorizedSQLException, DataAccessException areSolution
If you want to make the compiler spit out errors when the exceptions aren't caught, make your classes that derive from
You've got a bunch of random newlines that just make your code look ugly. I'd suggest removing all of them in the code you've posted.
Rather than having separate
one. The general form is this:
Also, the standard in Java is to put the opening brace on the same line as the statement it belongs to, so
should really be
Ignoring the wacky indentation and unconventional brackets, there are two tips I wanna give:
-
-
Of course, you can sidestep that whole issue by getting rid of the
Follow that, and your first method (with formatting) becomes just this:
And your second becomes (changing the whitespace to follow the majority of the
Exception, but not RuntimeException. Note that unless you have control over the source from whence they came, since you can't change the definition of classes, you have to create your own custom ones.You've got a bunch of random newlines that just make your code look ugly. I'd suggest removing all of them in the code you've posted.
Rather than having separate
catch blocks for each, you can put them all into one. The general form is this:
catch (ExceptionTypeA | ExceptionTypeB name) { /* ... */ }Also, the standard in Java is to put the opening brace on the same line as the statement it belongs to, so
catch (UncategorizedSQLException e)
{
return new ErrorMessage(errorInvalidQuery).toJson();
}should really be
catch (UncategorizedSQLException e) {
return new ErrorMessage(errorInvalidQuery).toJson();
}} catch (DataAccessException e) {
if (e instanceof UncategorizedSQLException)
{
//Invalid cypher query
throw (UncategorizedSQLException)e;
}
else throw e;
}Ignoring the wacky indentation and unconventional brackets, there are two tips I wanna give:
-
catch statements are done in order, using (what's basically) an instanceof check that stops after the first match. This means that if you put a superclass before a subclass, that subclass will never get hit -- so just put the subclass first:catch (UncatergorizedSQLException e) {
throw e;
}
catch (DataAccessException e) {
throw e;
}-
Of course, you can sidestep that whole issue by getting rid of the
try and declaring that your method throws those two exceptions -- since you're doing nothing but rethrowing, it's simpler and easier. Basically, since you've already declared that you're throwing, just remove the try and its associated catches.Follow that, and your first method (with formatting) becomes just this:
public String runQuery(String q) throws JsonGenerationException, JsonMappingException,
UncategorizedSQLException, DataAccessException {
Graph g = jdbcTemplate.query(q, new Neo4jGraphResultSetExtractor());
json = g.toJson(); //throws JsonGenerationException, JsonMappingException
return json;
}And your second becomes (changing the whitespace to follow the majority of the
catches):public String submitCypherQuery(String query) {
try {
return neoConn.runQuery(query);
} catch (UncategorizedSQLException e) {
return new ErrorMessage(errorInvalidQuery).toJson();
} catch (DataAccessException | ClassNotFoundException | JsonGenerationException |
IOException | JsonMappingException e) {
return handleUnexpectedError(e);
}
}Code Snippets
catch (ExceptionTypeA | ExceptionTypeB name) { /* ... */ }catch (UncategorizedSQLException e)
{
return new ErrorMessage(errorInvalidQuery).toJson();
}catch (UncategorizedSQLException e) {
return new ErrorMessage(errorInvalidQuery).toJson();
}} catch (DataAccessException e) {
if (e instanceof UncategorizedSQLException)
{
//Invalid cypher query
throw (UncategorizedSQLException)e;
}
else throw e;
}catch (UncatergorizedSQLException e) {
throw e;
}
catch (DataAccessException e) {
throw e;
}Context
StackExchange Code Review Q#95290, answer score: 5
Revisions (0)
No revisions yet.