• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Ron McLeod
  • Paul Clapham
  • Bear Bibeault
  • Junilu Lacar
Sheriffs:
  • Jeanne Boyarsky
  • Tim Cooke
  • Henry Wong
Saloon Keepers:
  • Tim Moores
  • Stephan van Hulst
  • Tim Holloway
  • salvin francis
  • Frits Walraven
Bartenders:
  • Scott Selikoff
  • Piet Souris
  • Carey Brown

Did I use Prepared Statements correctly?

 
Ranch Hand
Posts: 202
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I wrote the following method, but I'm not entirely sure if this is the correct way to implement a prepared statement... I don't feel entirely comfortable with it. I thought that I would have to implement "?" (question marks) which would tell `ps.setString(1, nickname);` which item in the query I am referring to...

 
Marshal
Posts: 15887
265
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
No, it's not the proper way. In fact, it's absolutely the wrong way.

See https://docs.oracle.com/javase/tutorial/jdbc/basics/prepared.html
 
Marshal
Posts: 25831
69
Eclipse IDE Firefox Browser MySQL Database
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Mark Richardson wrote:I don't feel entirely comfortable with it. I thought that I would have to implement "?" (question marks) which would tell `ps.setString(1, nickname);` which item in the query I am referring to.



Your feeling is correct. And so, yes, you should do that. But since you're asking the question, it looks like the way to do that isn't jumping out at you.

So here's how to do that. First of all, your query string shouldn't have anything but string constants in it. Ideally it's just one long string constant but until Java 15 brings us multi-line string constants you might want to break the one constant up into more than one part so that it doesn't go off the right-hand side of the screen, as your posted example does.

But it shouldn't be built from anything else. Not like your example, which includes the Java variable "nickname". That's (sort of) where the ? is going to go, but let's do it one step at a time. I'm going to use a simpler query than yours but it's the same principle. Like this: start with an ordinary query with no WHERE clause:


Now add in a WHERE clause with a constant value, so you can get a valid query:


Finally replace the constant value by the ? character which will mark the parameter location:


Note that the quotes around string constants in SQL are part of the constant, so don't leave them in when you put in the ? character. If you do, you'll get mystifying errors.
 
Sheriff
Posts: 22002
107
Eclipse IDE Spring VI Editor Chrome Java Ubuntu Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Note that you are allowed to use hard-coded values like 'C', 'F' and 'G', as you control their values and these values are safe. However, in this case you may want to replace them with some string constants that you then set on the prepared statement. The reason is that it may not be completely clear what 'C', 'F' and 'G' mean; you can give meaning by choosing a descriptive constant name (so don't use STATUS_G).
 
Saloon Keeper
Posts: 22503
151
Android Eclipse IDE Tomcat Server Redhat Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What about it is "absolutely the wrong way"? "It doesn't work" doesn't help us. Although I can see that the construct
Is probably over-engineered, since I'm pretty sure that a query is either going to return a non-null ResultSet or throw an Exception and thus doesn't need a test for null. But I'm too lazy to check the manual.

This is me:


And with that I see what was wrong with the original SQL - it wasn't in prepared form.

Some people might go even further and start each column name/phrase on a separate line, but that's a matter of preference - and how wide the display device you'll be reading the code is.
 
Junilu Lacar
Marshal
Posts: 15887
265
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Mark Richardson wrote:


Line 5 is the part that you got particularly wrong. Whenever you see string concatenation like this to build up a SQL query, think "SQL Injection Attack!"
 
Rob Spoor
Sheriff
Posts: 22002
107
Eclipse IDE Spring VI Editor Chrome Java Ubuntu Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Tim Holloway wrote:Although I can see that the construct
Is probably over-engineered, since I'm pretty sure that a query is either going to return a non-null ResultSet or throw an Exception and thus doesn't need a test for null. But I'm too lazy to check the manual.


You're right. There are three execute methods:

  • executeQuery either returns a ResultSet or throws an exception
  • executeUpdate either returns an int (for the number of updated records) or throws an exception
  • execute either returns a boolean or throws an exception. If true is returned, getResultSet() can be used, otherwise getUpdateCount() can be used (which returns -1 if there are no more results)
  •  
    Tim Holloway
    Saloon Keeper
    Posts: 22503
    151
    Android Eclipse IDE Tomcat Server Redhat Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Also, as a general rule, closing an outer construct for java.sql will close its inner constructs. That is, closing a Connection should close its outstanding Statements, which should close their outstanding ResultSets.
     
    Rob Spoor
    Sheriff
    Posts: 22002
    107
    Eclipse IDE Spring VI Editor Chrome Java Ubuntu Windows
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Rob Spoor wrote:

    Tim Holloway wrote:Although I can see that the construct
    Is probably over-engineered, since I'm pretty sure that a query is either going to return a non-null ResultSet or throw an Exception and thus doesn't need a test for null. But I'm too lazy to check the manual.


    You're right. There are three execute methods:

  • executeQuery either returns a ResultSet or throws an exception
  • executeUpdate either returns an int (for the number of updated records) or throws an exception
  • execute either returns a boolean or throws an exception. If true is returned, getResultSet() can be used, otherwise getUpdateCount() can be used (which returns -1 if there are no more results)

  • I just found out that we were both wrong. getResultSet() will return null if the result is an update count or there is no more result set. That last bit can happen if you execute multiple statements combined.
     
    It's a pleasure to see superheros taking such an interest in science. And this tiny ad:
    Thread Boost feature
    https://coderanch.com/t/674455/Thread-Boost-feature
      Bookmark Topic Watch Topic
    • New Topic