CODEWITHSUNDEEP
javaandroidarraylist
carl
carl

Reputation: 396

What is wrong with my insert method?

I am trying to insert some data to a database. To achieve this i made a method that returns a boolean and receives an ArrayList of Objects of type Point. Everything runs fine until i checked my table and i discovered that it is just one point which has been inserted(the first point each time) although i have a plenty of points(arrayList.size() > 0). I can reach for example arrayList.get(2).getXcoordinate()if my arrayList.size(2), but my method insert just the first index(arrayList.size(0)). I simply can not figure out where i am doing wrong. Any help or hints is very appreciated. Thanks, Carl - My method:

public boolean insertPoints(ArrayList<Point> arr) {
       try {
            con = this.getConnection();
            st = con.createStatement();
            if (con != null) {
                for (int i = 0; i < arr.size(); i++) {
                    String id = arr.get(i).getId();
                    String x = arr.get(i).getXcoordinate();
                    String y = arr.get(i).getYcoordinate();
                    if (st.executeUpdate("INSERT INTO table (Id, X, Y)
                                 VALUES (" + id + "," + x + "," + y + ")") > 0)
                        return true;
                    return false;
                }
            } else {
                System.err.println("No connection, con == null ...");
                return false;
            }
        } catch (Exception e) {
            System.err.println("Exception: " + e.getMessage());
            return false;
        } finally {
            try {
                if (rs != null) rs.close();
                if (st != null) st.close();
                if (con != null) con.close();
            } catch (SQLException sqle) {
                System.err.println("SQLException: " + sqle.getMessage());
            }
        }
         return false;
     }

Upvotes: 1

Views: 84

Answers (4)

Pittmyster
Pittmyster

Reputation: 62

Try this:

public boolean insertPoints(ArrayList<Point> arr)
    {
        try
        {
            con = this.getConnection();
            st = con.createStatement();
            if (con != null)
            {
                boolean success = true;
                for (int i = 0; i < arr.size(); i++)
                {
                    String id = arr.get(i).getId();
                    String x = arr.get(i).getXcoordinate();
                    String y = arr.get(i).getYcoordinate();
                    if (st.executeUpdate("INSERT INTO table (Id, X, Y) VALUES (" + id + "," + x + "," + y + ")") > 0)
                    {
                        continue;
                    }
                    else
                    {
                        success = false;
                        break;
                    }
                }
                return success;
            }
            else
            {
                System.err.println("No connection, con == null ...");
                return false;
            }
        }
        catch (Exception e)
        {
            System.err.println("Exception: " + e.getMessage());
            return false;
        }
        finally
        {
            try
            {
                if (rs != null) rs.close();
                if (st != null) st.close();
                if (con != null) con.close();
            }
            catch (SQLException sqle)
            {
                System.err.println("SQLException: " + sqle.getMessage());
            }
        }
        return false;
    }

Upvotes: -1

duffymo
duffymo

Reputation: 308813

This code is not the way you should do this.

Please use PreparedStatement to bind variables.

Your Connection and PreparedStatement should not be class members. The Connection should be instantiated elsewhere and passed in.

The PreparedStatement should have local scope.

You should close resources in individual try/catch blocks in the finally method. Write static methods to reuse and keep the code small.

You might also consider batching these calls so you INSERT all points in a single round trip. It'll be more performant.

// Your table's name is table?  Terrible.
private static final String INSERT_SQL = "INSERT INTO table(Id, X, Y) VALUES (?, ?, ?)";

Upvotes: 2

kavi temre
kavi temre

Reputation: 1321

this is problematic

if (st.executeUpdate("INSERT INTO table (Id, X, Y)
                                 VALUES (" + id + "," + x + "," + y + ")") > 0)
                        return true;
                    return false;

just remove both the return statement and use a flag or counter

Upvotes: 1

George
George

Reputation: 915

The following:

if (st.executeUpdate("INSERT INTO table (Id, X, Y) VALUES (" + id + "," + x + "," + y + ")") > 0)
    return true;
return false;

will exit the method after inserting the first value, returning either true or false based on the return of executeUpdate.

Maybe something more like this instead

 for (int i = 0; i < arr.size(); i++) {
      String id = arr.get(i).getId();
      String x = arr.get(i).getXcoordinate();
      String y = arr.get(i).getYcoordinate();
      if (!st.executeUpdate("INSERT INTO table (Id, X, Y) VALUES (" + id + "," + x + "," + y + ")") > 0)
          return false;
 }
 return true;

Upvotes: 1

Related Questions