2017-04-25 2 views
2

私はJavaに慣れていません。私の仕事はすべてJDBCに関連しています - データの挿入と処理についてです。すべての上に働いて罰金。単一の試行キャッチブロック内の複数のJDBC文。それはグッドプラクティスですか?

コードを減らすには、try{} catch()ブロックを複数使用してJDBCStatementsPrepared Statementsと書くことができます。

コード例:

public void dashboardReports() 
{ 
    try { 

     String total_stock_value="select sum(price*closingstock)as tsv from purchase_table"; 
     Statement ps_tsv=connection.createStatement(); 
     ResultSet set_tsv=ps_tsv.executeQuery(total_stock_value); 
     if(set_tsv.next()) 
     { 
      total_stock.setText(set_tsv.getString("tsv"));    
     }   

     String tota_sales="select sum(INVOICE_VALUE) as iv from PARTYWISE_ACCOUNTS_LEDGER"; 
     Statement st_total_sales=connection.createStatement(); 
     ResultSet set_total_sales=st_total_sales.executeQuery(tota_sales); 
     if(set_total_sales.next()) 
     { 
      total_sales.setText(set_total_sales.getString("iv")); 
     }   

     String total_purchases="select sum(CP_INVOICEVALUE)as cpi from COMPANY_PAYMENTS"; 
     Statement st_tps=connection.createStatement(); 
     ResultSet set_tps=st_tps.executeQuery(total_purchases); 
     if(set_tps.next()) 
     { 
      total_purchases_label.setText(set_tps.getString("cpi")); 
     } 

     String total_collectionss="select sum(PAYMENT_REC) as payrec from PARTYWISE_ACCOUNTS_LEDGER"; 
     Statement ps_toco=connection.createStatement(); 
     ResultSet set_toco=ps_toco.executeQuery(total_collectionss); 
     if(set_toco.next()) 
     { 
      total_collections.setText(set_toco.getString("payrec")); 
     } 

     String total_payments="select sum(CP_PAYMENTREC) as paid from COMPANY_PAYMENTS"; 
     Statement ps_topa=connection.createStatement(); 
     ResultSet set_topa=ps_topa.executeQuery(total_payments); 
     if(set_topa.next()) 
     { 
      total_payments_label.setText(set_topa.getString("paid")); 
     } 

    } catch (Exception e) { 
     // TODO: handle except 
    } 
} 

だから、ハンドルに、この良い方法または別の他の方法はありますか?

現在のところ、私のコードは非常にうまく動作していますが、この種のアプローチでは今後の問題がありますか?

+1

このコードが正常に動作している場合、あなたが私たちの[コードレビュー](HTTPS上でそれを提出しなければなりません/codereview.stackexchange.com/)姉妹サイト。 –

+0

私が見ている最大の問題は、問題の少ないリソースを試してみるとリソースを閉じないということです。小さなスコープのtryブロックを自動的に使用することになります。ところで、あなたのコードは非効率的です。あなたは2つのテーブルを2回クエリしています。 –

答えて

2

ここではSELECT操作しか行っていないので、データベースの状態は変更されておらず、ロールバックすることもないため、明示的なトランザクションは本当に必要ありません。 1つのtryブロック内にすべてのSELECTステートメントをグループ化することは何も問題ありません。しかし、潜在的な欠点があります。つまり、SELECTが1つ失敗した場合、そのコードはブロックtryを終了し、その後のすべてのクエリは実行されません。もしあなたがこれを容認できれば、あなたはそのままあなたを残すことができます。これの類推は、連続して接続された一連の電球であろう。 1つが壊れると、彼らはすべて出かける。

クエリには別々のtryブロックを使用することをお勧めします。そして、たとえその中で例外が発生しても、他のものが正常に完了する可能性があります。ここでの類推は、並列回路における一連の電球であろう。

0

ちょうどあなたがあなたの文と結果セットを閉じます

try { 

    String total_stock_value="select sum(price*closingstock)as tsv from purchase_table"; 
    try (Statement ps_tsv=connection.createStatement(); 
     ResultSet set_tsv=ps_tsv.executeQuery(total_stock_value)) { 
     if(set_tsv.next()) 
     { 
      total_stock.setText(set_tsv.getString("tsv")); 
     } 
    } 

    String tota_sales="select sum(INVOICE_VALUE) as iv from PARTYWISE_ACCOUNTS_LEDGER"; 
    try (Statement st_total_sales=connection.createStatement(); 
      ResultSet set_total_sales=st_total_sales.executeQuery(tota_sales)) { 
     if(set_total_sales.next()) 
     { 
      total_sales.setText(set_total_sales.getString("iv")); 
     } 
    } 

    String total_purchases="select sum(CP_INVOICEVALUE)as cpi from COMPANY_PAYMENTS"; 
    try (Statement st_tps=connection.createStatement(); 
      ResultSet set_tps=st_tps.executeQuery(total_purchases)) { 
     if(set_tps.next()) 
     { 
      total_purchases_label.setText(set_tps.getString("cpi")); 
     } 
    } 

    String total_collectionss="select sum(PAYMENT_REC) as payrec from PARTYWISE_ACCOUNTS_LEDGER"; 
    try (Statement ps_toco=connection.createStatement(); 
      ResultSet set_toco=ps_toco.executeQuery(total_collectionss)) { 
     if(set_toco.next()) 
     { 
      total_collections.setText(set_toco.getString("payrec")); 
     } 
    } 

    String total_payments="select sum(CP_PAYMENTREC) as paid from COMPANY_PAYMENTS"; 
    try (Statement ps_topa=connection.createStatement(); 
      ResultSet set_topa=ps_topa.executeQuery(total_payments)) { 
     if(set_topa.next()) 
     { 
      total_payments_label.setText(set_topa.getString("paid")); 
     } 
    } 

} catch (Exception e) { 
    // TODO: handle except 
} 

}

1

を使用すると、1つに障害が発生した場合、その後、私は例外をスローするメソッドを変更します失敗し、以降のすべてのSELECT秒に満足している場合

public void dashboardReports() throws SQLException 
{ 
.... 
} 

呼び出しメソッドからSQLExceptionをキャッチします。

私はSQLExceptionではなく、私はあなたのコードはOKだと思いますException

+0

ちょうど私が書いたものに類似したビット...フィードバックはそこに大歓迎です。 – GhostCat

2

をキャッチ/スローする方が良いと思います。 最終的な結果セット、ステートメント、および接続がブロックで最後に必要です。あなたがトライcatchブロックで囲んで、このメソッドを呼び出すと

public String execute(String query) throws SQLException { 
Statement ps_toco=connection.createStatement(); 
     ResultSet set_toco=ps_toco.executeQuery(query); 
     return set_toco.next(); 
} 

0

より良い方法は、一般的な操作を行うメソッドを作成することです。

0

1.コードの行をカプセル化して減らすために、executeQuery(Connection conn, Statement st, String sql)のようなメソッドを使用できます。

2.Don'tは、SQL特定の例外クラスすぎ

3をキャッチし、一般的なExceptionに依存しています。finallyブロックが存在しないため、リソースを適切に閉じることができません。また、あなたはcatchブロックに何をする必要があるかfinallyブロック

4.Seeの必要性を排除するためにtry with resource構文を使用して試すことができます - あなたは、チェーン上の例外を伝播するか、右がプログラムを失敗する必要がありますか?

5.私の意見では、ResultSetStatementはできるだけ早く閉じてみる必要があります。できるだけ早くそれらを閉じてみてください。ポイント1は、これを達成するのに役立ちます。

技術的な正確さと妥当性の観点からは、すべてのSQLに対して1つのtry-catchを使用して例外を除外してコードを書くことには害はありませんが(私はSELECT sqlsがそこにしかないので) 、読み取り可能で保守可能なコードであり、そのコードではコードが貧弱に見えます。

2

これはSingle ResponsbilitySingle Layer of Abstractionの原則に違反します。

このコードは技術的には有効ですが、そのの正確さに焦点を当てるだけでなく、読みやすさにも焦点を当てるべきです。そしてテスト容易性。そして私はどちらも、あなたが見せているインプットに「素晴らしい」とは思わないと思います。

したがって、クリーンなコード(品質)の観点から来る;あなたがそこに着いた別の例ごとに、小さなヘルパーと

outer method ... 
    try { 
    helperMethod1(); 
    helperMethod2(); 
    } catch(... 

:私はむしろの線に沿って何かのために行くために助言します。もちろん、あなたはそこで止まらないでしょう。それらのヘルパーの側面を共通ののように分離しようとする。たぶん単一の、より一般的なヘルパーと一緒に行く方法を見つけることができます。

もちろん:可能であれば例外をキャッチしないようにしてください。代わりに、あなたは最も具体的な例外をキャッチ!

+2

+1私はあなたが言っていることに同意し、 'helperMethod1'と' helperMethod2'が機能的にほとんど同じであるため、追加することもできます。 –

+0

素晴らしいアイデアです。私の答えにそれを加えた。 – GhostCat

0

あなたのコードは動作しますが、私は以下のように、より良いメンテナンスと可読性のためにをリファクタリングすることを強くお勧めします。また、リソースが適切に閉じられていることを確認します。

public void dashboardReports() { 
    handleTotalStocks(); 
    handleTotalSales(); 
    handleTotalPurchages(); 
    //Add others 
} 

handleTotalStocks()メソッド:/:

private void handleTotalStocks() { 
     String total_stock_value="select sum(price*closingstock)as tsv 
        from purchase_table"; 

    try(Statement ps_tsv=connection.createStatement(); 
     ResultSet set_tsv=ps_tsv.executeQuery(total_stock_value);) { 
     if(set_tsv.next()) { 
      total_stock.setText(set_tsv.getString("tsv")); 
     } 
     } 
    } 
//add other methods 
関連する問題