2017-02-02 3 views
1

これは私のコードです。プログラムの行数を減らす方法があるかどうかを知る必要があります。条件のメソッド呼び出しで多くのifステートメントを持つコード行を減らすには、より良い方法が必要です

if文にはメソッド呼び出しが含まれているため、それらを置き換えるためにenumを使用することはできません。あなたのコードで

 
public class AgreementQueryBuilder { 

    @SuppressWarnings("unchecked") 
    public static String searchQueryBuilder(FetchAgreementsModel agreementsModel, 
      @SuppressWarnings("rawtypes") List obj) { 

     StringBuilder selectQuery = new StringBuilder(
       "select *,e.id as agreementid from eglams_agreement e left outer join eglams_rentincrementtype t on e.rent_increment_method=t.id where"); 

     // if statement to check the arguments and build the where criteria 

     if (agreementsModel.getAgreementId() != null || agreementsModel.getAgreementNumber() != null 
       || agreementsModel.getStatus() != null || agreementsModel.getTenantId() != null 
       || (agreementsModel.getFromDate() != null && agreementsModel.getToDate() != null) 
       || agreementsModel.getTenderNumber() != null || agreementsModel.getTinNumber() != null 
       || agreementsModel.getTradeLicense_number() != null) 
      return null; 

     boolean isAllFieldsNull = true; 

     if (agreementsModel.getAgreementId() != null) { 
      if (isAllFieldsNull == true) { 
       selectQuery.append(" e.id=?"); 
       isAllFieldsNull = false; 
       obj.add(agreementsModel.getAgreementId()); 
      } 
     } 

     if (agreementsModel.getAgreementNumber() != null) { 
      if (isAllFieldsNull == true) { 
       selectQuery.append(" e.agreement_number=?"); 
       isAllFieldsNull = false; 
      } else 
       selectQuery.append(" and e.agreement_number=?"); 
      obj.add(agreementsModel.getAgreementNumber()); 
     } 

     if (agreementsModel.getStatus() != null) { 

      if (isAllFieldsNull == true) { 
       selectQuery.append(" e.status=?"); 
       isAllFieldsNull = false; 
      } else 
       selectQuery.append(" and e.status=?"); 

      obj.add(agreementsModel.getStatus()); 
     } 

     if (agreementsModel.getTenantId() != null) { 
      if (isAllFieldsNull == true) { 
       selectQuery.append(" e.tenant_id=?"); 
       isAllFieldsNull = false; 
      } else 
       selectQuery.append(" and e.tenant_id=?"); 

      obj.add(agreementsModel.getTenantId()); 
     } 

     if (agreementsModel.getTenderNumber() != null) { 
      if (isAllFieldsNull == true) { 
       selectQuery.append(" e.tender_number=?"); 
       isAllFieldsNull = false; 
      } else 
       selectQuery.append(" and e.tender_number=?"); 
      obj.add(agreementsModel.getTenderNumber()); 
     } 
     if (agreementsModel.getTinNumber() != null) { 
      if (isAllFieldsNull == true) { 
       selectQuery.append(" e.tin_number=?"); 
       isAllFieldsNull = false; 
      } else 
       selectQuery.append(" and e.tin_number=?"); 
      obj.add(agreementsModel.getTinNumber()); 
     } 

     if (agreementsModel.getTradeLicense_number() != null) { 
      if (isAllFieldsNull == true) { 
       selectQuery.append(" e.TradeLicense_number=?"); 
       isAllFieldsNull = false; 
      } else 
       selectQuery.append(" and e.TradeLicense_number=?"); 
      obj.add(agreementsModel.getTradeLicense_number()); 
     } 

     if (agreementsModel.getFromDate() != null) { 
      if (agreementsModel.getToDate() != null) { 
       if (isAllFieldsNull == true) { 
        selectQuery.append(" t.FromDate=?"); 
        isAllFieldsNull = false; 
       } else 
        selectQuery.append(" and t.FromDate=?"); 

       obj.add(agreementsModel.getFromDate()); 
       selectQuery.append(" and t.ToDate=?"); 
       obj.add(agreementsModel.getToDate()); 
      } 
     } 
     System.err.println(selectQuery); 
     return selectQuery.toString(); 
    } 
} 
+4

ようこそスタックオーバーフロー!あなたのコードは現在動作しているようですが、あなたはそれを改善しようとしています。一般的に、これらの質問はこのサイトでは強すぎますが、[CodeReview.SE](// codereview.stackexchange.com/tour)のほうが良いかもしれません。このサイトよりも少し厳密であるため、[必要条件](// codereview.stackexchange.com/help/on-topic)を必ずお読みください。 – 4castle

+0

すべてのwhere句をリストに入れます。それらを ''と ''と最後に結びつけてください。 –

答えて

1

ルック:

if (agreementsModel.getAgreementId() != null || ...) 
    return null; 

だから我々はgetAgreementId() != null場合、このメソッドはnullを返しますし、任意のさらなる行くことはありませんことを知っています。しかし、その直後、あなたは書く:

if (agreementsModel.getAgreementId() != null) { 
     selectQuery.append(" e.id=?"); 
     isAllFieldsNull = false; 
     obj.add(agreementsModel.getAgreementId()); 
    } 
} 

getAgreementId() != nullが本当だった場合、この方法はすでに戻っていたので、我々は、このブロック全体を入力することはできませんことを知っています。

あなたの質問への答えは「はい」です。決して呼び出されない無関係なコードブロックをすべて削除することで、このコードを短くすることができます。

長い答えは、コードを修正してデバッグして、コードを短くすることができるかどうかを心配する前に実際に行うことを期待することです。

if (agreementsModel.getAgreementId() != null || agreementsModel.getAgreementNumber() != null 
      || agreementsModel.getStatus() != null || agreementsModel.getTenantId() != null 
      || (agreementsModel.getFromDate() != null && agreementsModel.getToDate() != null) 
      || agreementsModel.getTenderNumber() != null || agreementsModel.getTinNumber() != null 
      || agreementsModel.getTradeLicense_number() != null) 
     return null; 

しかし、唯一のあなたのコードがどのように動作するかを知ることができます。私はそれはそれは、以下のすべてのブロックが無駄になりますので、このifを除去することができることを意味疑い。私はそれを短くする方法を尋ねる前に、それが動作することを確認するためにテストすることを提案します。

1

SQLクエリを生成するために条件が使用されていることがわかりました。 MyBatis/Hibernateのようなマッパーフレームワークを使用して、これをより読みやすく、短く保守しやすいコードに変更する必要があると思います。以下Mybatisを使用して条件付きでクエリの

サンプル:上記のサンプルで

<select id="findActiveBlogLike" resultType="Blog"> 
    SELECT * FROM BLOG WHERE state = ‘ACTIVE’ 
    <if test="title != null"> 
     AND title like #{title} 
    </if> 
    <if test="author != null and author.name != null"> 
     AND author_name like #{author.name} 
    </if> 
    <!-- add another conditional to add extra filter --> 
</select> 

、我々は動的に入力された豆のブログをチェックして、クエリを生成することができます。 titleがnullでない場合は、条件にタイトルを追加し、authoerがnullでない場合は、条件にも著者を追加します。

0

あなたのコードはすべて同じパターンに従って

ある
if (agreementsModel.getAgreementNumber() != null) { 
    if (isAllFieldsNull == true) { 
    selectQuery.append(" e.agreement_number=?"); 
    isAllFieldsNull = false; 
    } else 
    selectQuery.append(" and e.agreement_number=?"); 

     obj.add(agreementsModel.getAgreementNumber()); 
} 

:何かがnullでない場合、リストにその事を置きます。いくつかのフラグに基づいて文字列を追加します。フラグをtrueに設定します。

あなたはメソッドとしてこれを書くことができます。

boolean yourMethod(Object obj, List<Object> objs, String clause, StringBuilder clauses, boolean flag) { 
    if (obj == null) return flag; 

    objs.add(obj); 
    if (!flag) clauses.append(" and "); 
    clauses.append(clause); 

    return false; 
} 

それで、あなたは今とあなたの上記のコードスニペットを置き換えることができます。

isAllFieldsNull = yourMethod(
    agreementsModel.getAgreementNumber(), 
    obj, 
    " e.agreement_number=? ", 
    selectQuery, 
    isAllFieldsNull); 

など、以降のフィールドのために。

関連する問題