Recording a customer's shopping cart as an order

Question

I have come up with the following method to post a transaction to a database, in the context of an e-commerce Java web application. The transaction consists in submitting a new order alongside its details:

public long insertOrder(Customer customer, Hashtable<String, CartItem> shoppingCart)
{
    long returnValue = 0L;
    Connection connection = null;
    Statement statement = null;
    try {
        connection = MySqlConnection.getConnection();
        connection.setAutoCommit(false);
        statement = connection.createStatement();
        long orderId = System.currentTimeMillis();
        OrderPeer.insertOrder(statement, orderId, customer);
        OrderDetailsPeer.insertOrderDetails(statement, orderId, shoppingCart);
        connection.commit();
        returnValue = orderId;
    } catch (SQLException e) {
        try { connection.rollback(); } catch (Exception ex) { }
        System.out.println("Could not insert order: " + e.getMessage());
    } finally {
        try { statement.close(); } catch (Exception e) { }
        try { connection.close(); } catch (Exception e) { }
    }
    return returnValue;
}

I would like to know if I am handling the exception, rolling back, and closing the statement/connection in the best possible manner.

Answer 1

Using the "try-with-resources" statement should clean up your code significantly. There are two advantages:

1. It allows you to declare your resources (in this case Connection and Statement objects) in exactly the scope they're going to be used. This means you don't have to first initialize them to null, which is ugly.
2. It will ensure your resources get closed no matter what.

Also, you should probably print the stack trace (to aid you and others in debugging), and you should print your error messages to stderr instead of stdout.

public long insertOrder(Customer customer, Hashtable<String, CartItem> shoppingCart)
{
    try (Connection connection = MySqlConnection.getConnection();
        Statement statement = connection.createStatement();) {
        connection.setAutoCommit(false);
        long orderId = System.currentTimeMillis();
        OrderPeer.insertOrder(statement, orderId, customer);
        OrderDetailsPeer.insertOrderDetails(statement, orderId, shoppingCart);
        connection.commit();
        return orderId;
    } catch (SQLException e) {
        try {
           connection.rollback();
        } catch (Exception ex) {
            System.err.println("Could not roll back connection: " + ex.getMessage())
            ex.printStackTrace();

        }
        System.err.println("Could not insert order: " + e.getMessage());
        e.printStackTrace()l
        return 0;
    }
}

Answer 2

Instead of returning long, with zero representative of either a failure to insert or a failed rollback, suggest returning objects instead. Objects could be created for narrower range of order id's, objects which had instance variable area to store specific success or failure details or provide useful predicate methods: isInsertOK, isRollback, etc.

Would be safer not to use long without any range. Risk may be unlikely now as method is small but suppose some error or corruption occurrs resulting in an invalid order id but valid long. These objects would consolidate repeated procedural code handling return codes, order id's, failures.