Поведение вставки DAO не кажется нормальным - PullRequest
Новая
       8

Поведение вставки DAO не кажется нормальным

0 голосов
/ 14 февраля 2020

Я сделал DAO, чтобы создать коммерческое предложение, и оно похоже на другое DAO, где он создает пользователей, который подключается и работает. Но в приведенном ниже примере пропускаются операторы if, и я не уверен, почему он не добавляется в базу данных. Я запустил ту же команду в строке SQL в oracle, и она сработала там. 

public boolean sendOffer(Sales sell) {

    boolean done = false;
    int key =0;
    Connection conn = cu.getConnection();
    try {

        String sql = "INSERT INTO sales (offer_amount, offer_who, car_id) values(?,?,?)";
        String[]keys= {"ID"};
        PreparedStatement ps = conn.prepareStatement(sql,keys);
        ps.setInt(1, sell.getOfferAmount());
        ps.setInt(2, sell.getOwnerID());  //foriegn key
        ps.setInt(3, sell.getCarID()); // forgien key
        int number = ps.executeUpdate();
        ResultSet rs = ps.getGeneratedKeys();
        if(number!=1) 
        {
            log.warn("data insert fail");
        }
        else 
        {
            log.trace("success");
            done=true;
        }
           if(rs.next()) {
               key=rs.getInt(1);
               sell.setID(key);
               conn.commit();
               }
           else {
               log.warn("data not found");
           }


    }catch(SQLException e)
    {

    }
finally {
    try {
        conn.close();
    } catch (SQLException e) {
        // TODO Auto-generated catch block
        //e.printStackTrace();
    }
}

    // TODO Auto-generated method stub
    return done;
}'''

1 Ответ

1 голос
/ 14 февраля 2020

Основная проблема заключается в том, что в вашем коде происходит исключение, но блок try / catch перехватывает его и молча проглатывает. Хотя может быть заманчиво перехватывать и глотать исключения, правда заключается в том, что они всегда вызывают больше проблем, чем решают, и ключевая концепция обработки исключений - НЕ обрабатывать их: просто поместите объявление throws и позвольте приложению cra sh.

Затем у вас возникают другие возможные побочные проблемы в зависимости от того, как Connection был получен в первую очередь, например, тот факт, что вы никогда не закрываете PreparedStatement и ResultSet (если соединение закрыто, они также закрыты ... но если соединение возвращается в пул, то они никогда не будут закрыты).

В общем, приведенный выше код имеет тенденцию упаковывать слишком много функциональности в одном метод и может быстро выйти из-под контроля, поэтому вы можете разделить его на более мелкие куски с четкими индивидуальными обязанностями. будь то из соображений максимальной производительности или из-за отсутствия опыта. Обычно в веб-приложениях, использующих среду Spring, это решается с помощью JdbcTemplate, но я не могу предположить, что вы используете Spring, поэтому я не буду здесь показывать его использование.

Как минимум, я бы измените ваш код примерно следующим образом: 

public boolean sendOffer(Sales sell) {
    Connection conn = null;
    PreparedStatement ps = null;
    ResultSet rs = null;
    try {
        conn = cu.getConnection();
        ps = prepareInsertOfferStatement(sell, conn);
        ps.executeUpdate();
        rs = ps.getGeneratedKeys();
        sell.setID(extractKey(rs));
        conn.commit();
        log.trace("success");
        return true;
    }
    catch(Exception ex) {
        log.error(ex);  // this is actually probably bad. Consider putting a throws declaration and get rid of this catch
        return false;
    }
    finally {
        closeQuietly(rs, ps, conn);
    }
}

private void closeQuietly(AutoCloseable... objs) {
    for(AutoCloseable obj : objs) {
        try {
            obj.close();
        } catch (SQLException e) {
            // this is usually mostly safe to ignore. Maybe log a warning somewhere
        }
    }
}

private PreparedStatement prepareInsertOfferStatement(Sales sell, Connection conn) throws SQLException {
    String sql = "INSERT INTO sales (offer_amount, offer_who, car_id) values(?,?,?)";
    String[] keys= {"ID"};
    PreparedStatement ps = conn.prepareStatement(sql,keys);
    ps.setInt(1, sell.getOfferAmount());
    ps.setInt(2, sell.getOwnerID());  //foreign key
    ps.setInt(3, sell.getCarID()); // foreign key
    return ps;
}

private int extractKey(ResultSet rs) throws SQLException {
    if(rs.next()) {
        return rs.getInt(1);
    }
    else {
        throw new Exception("The statement did not return any generated key.");
    }
}

Как видите, он не короче, но обязанности более понятны, и все объекты соответственно закрыты. Кроме того, он дает вам хорошие многократно используемые примитивы для закрытия соединений и связанных объектов, а также для извлечения ключа из других вставок, которые вы захотите сделать. Дальнейшие абстракции позволят вам получить больше примитивов, но я думаю, что этого достаточно, чтобы вы поняли суть этого.

...