2

I'm passing my method InsertQuery variables from another method which are entered by the user via Scanner.

How do I fill in the iName, iType etc. into my iQuery so that I can insert them into my DB?

public void InsertQuery (String iName, String iType, int health_Problem, Date date2, String aRemind, String docName, String docType, String docAdress) { final String url = "jdbc:mysql://localhost/ehealthdb?serverTimezone=UTC"; final String DBUSER = "root"; final String DBPSWD = "root"; try { Connection con = DriverManager.getConnection(url,DBUSER,DBPSWD); Statement stmt = con.createStatement(); String iQuery = "INSERT into appointment" + "(ID, PatientID, Insurance_Name, Insurance_Type, Health_Problem, Appointment_Date, Appointment_Remind, Doctor_Name,Doctor_Type,Doctor_Adress)" + "values ('1','1',,'Gesetzlich','5','15.01.2020','1 Week','Musterarzt','Hausarzt','Musterstraße')"; stmt.executeUpdate(iQuery); } catch (Exception e) { System.out.println("Something went wrong @InsertQuery"); } } 
3
  • 4
    Consider a PreparedStatement. Commented Jan 1, 2021 at 22:06
  • 1
    also consider this code leaks the database connection. using try-with-resources like this answer will fix that. also follow the exception handling advice here. Commented Jan 1, 2021 at 22:29
  • By the way, Date is a terrible class that was supplanted years ago by the modern java.time classes defined in JSR 310. Furthermore, appointments should be represented not as a moment, in case politicians change time zone's offset. Better to store LocalDateTime via PreparedStatement#setObject, and ZoneId as text. Commented Jan 1, 2021 at 22:57

4 Answers 4

5

The easiest approach would probably be to use a PreparedStatement:

public void insertQuery (String iName, String iType, int healthProblem, Date date2, String aRemind, String docName, String docType, String docAddress) throws SQLException { final String url = "jdbc:mysql://localhost/ehealthdb?serverTimezone=UTC"; final String DBUSER = "root"; final String DBPSWD = "root"; try (Connection con = DriverManager.getConnection(url,DBUSER,DBPSWD); PreparedStatement stmt = con.prepareStatement( "INSERT into appointment" + "(ID, PatientID, Insurance_Name, Insurance_Type, Health_Problem, Appointment_Date, Appointment_Remind, Doctor_Name, Doctor_Type, Doctor_Adress) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)")) { stmt.setString(1, iName); stmt.setString(2, iType); stmt.setInt(3, healthProblem); stmt.setTimestamp(4, new Timestamp(date2.getTime())); stmt.setString(5, aRemind); stmt.setString(6, docName); stmt.setString(7, docType); stmt.setString(8, docAddress); stmt.executeUpdate(); } } 
Sign up to request clarification or add additional context in comments.

5 Comments

Instead of System.out.println("Something went wrong @InsertQuery"); inside catch block, I suggest using e.printStackTrace(); so that the programmer can get to know the root cause of the problem.
@LiveandLetLive Personally, I'd just throw the exception upwards and let a part of the program with more context deal with it intelligently, but TBH, I just copied the catch block from the OP, as it's not the focus of this question
@Mureinik a ton of confusion in those new to java programming (source: Lots of things, including StackOverflow questions) arise from bad exception handling. Examples, blogs, and SO answers are perpetuating this error. I think it is time we as a community step up and start the hard work on eliminating this confusion. I hope you agree and start fixing catch blocks even if the question isn't about it, especially if it's fairly obvious it's a newish java programmer asking.
@Mureiknik and yes, of course - throwing that exception onwards is clearly the right move here. The method is named insertQuery, SQL is baked into its very definition, thus throws SQLException is warranted, no?
@rzwitserloot yeah, I guess that would be the responsible thing to do. Edited my answer accordingly.
2

Don't use a statement, use a PreparedStatement. Otherwise, you get hacked.

More generally, JDBC is a tricky beast and not a particularly nice API. For fairly good reasons - it is designed to be the lowest common denominator, and it is more focused on exposing all the bells and whistles of all databases in existence, than in giving you, programmer who wants to interact with a database, a nice experience.

Try JDBC or JOOQ.

Your exception handling is also wrong. If you catch an exception, either handle it, or make sure you throw something. Logging it, (or worse, printing it) definitely does not count. Add throws to your method signature. If that's not possible (and it usually is possible, try that first), throw new RuntimeException("Uncaught", e) is what you want. not e.printStackTrace(), or even worse, what you did: You just tossed out all relevant information. Don't do that.

Comments

2

The recommended approach is to use PreparedStatement which solves the following two important problems apart from many other benefits:

  1. It helps you protect your application from SQL Injection.
  2. You will not have to enclose the text values within single quotes yourself.

Typical usage is as shown below:

String query = "INSERT INTO appointment(ID, PatientID, Insurance_Name, Insurance_Type, Health_Problem) VALUES (?, ?, ?, ?, ?)"; try (PreparedStatement pstmt = con.prepareStatement(query)) { //... pstmt.setString(1, id); pstmt.setString(2, patientId); pstmt.setString(3, insuranceName); //... pstmt.executeUpdate(); } catch(SQLException e) { e.printStackTrace(); } 

Note that for each ?, you will have to use pstmt.setXXX. Another thing you need to understand is that in the method call, pstmt.setString(1, Id), 1 refers to the first ? and not the first column in your table.

Some other important points:

  1. I have used try-with-resources statement which is an easier and recommended way to close the resources after the program is finished with it. Learn more about it from Oracle's tutorial on it.
  2. Always follow Java naming conventions e.g. Insurance_Name should be named as insuranceName.

2 Comments

it might help to highlight the use of the try-with-resource construct since OP doesn't close any of the used resources
@fantaghirocco - Good suggestion. I have added a link to Oracle's tutorial on it.
-1

I used this way and it is working greatly

  • for iName
public void InsertQuery (String iName, String iType, int health_Problem, Date date2, String aRemind, String docName, String docType, String docAdress) { final String url = "jdbc:mysql://localhost/ehealthdb?serverTimezone=UTC"; final String DBUSER = "root"; final String DBPSWD = "root"; try { Connection con = DriverManager.getConnection(url,DBUSER,DBPSWD); Statement stmt = con.createStatement(); String iQuery = "INSERT into appointment" + "(ID, PatientID, Insurance_Name, Insurance_Type, Health_Problem, Appointment_Date, Appointment_Remind, Doctor_Name,Doctor_Type,Doctor_Adress)" + "values ('1','1',,'"+iName+"','5','15.01.2020','1 Week','Musterarzt','Hausarzt','Musterstraße')"; stmt.executeUpdate(iQuery); } catch (Exception e) { System.out.println("Something went wrong @InsertQuery"); } } 

1 Comment

This solution is unsafe. Do not concatenate values into a query string, it makes your code vulnerable to SQL injection. Instead, use a prepared statement with parameters, as demonstrated by some of the other answers.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.