switch文の循環的複雑度を削減 - ソナー

Question

私は私のスイッチケース 私のコードがあるのサイクロマティック複雑減らしたい：

public String getCalenderName() { 
     switch (type) { 
    case COUNTRY: 
     return country == null ? name : country.getName() + HOLIDAY_CALENDAR; 
    case CCP: 
     return ccp == null ? name : ccp.getName() + " CCP" + HOLIDAY_CALENDAR; 
    case EXCHANGE: 
     return exchange == null ? name : exchange.getName() + HOLIDAY_CALENDAR; 
    case TENANT: 
     return tenant == null ? name : tenant.getName() + HOLIDAY_CALENDAR; 
    default: 
     return name; 
    } 
} 

このコードブロックの複雑さは、16で、10 国にそれを減らしたいがccp、交換とテナントは私の別のオブジェクトです。タイプに基づいて、それぞれのメソッドを呼び出します。

Answer 1

私の知る限り、switch文ではreturn文を使用しないでください。変数を使用してswitchステートメントの後にそのロジックを適用します。 null値をチェックするメソッドを作成し、スイッチからそのメソッドを呼び出すと、循環性の複雑さを減らすことができます

Answer 2

私はそれがSonarと信じています。私はSonarの警告は必須のルールではなく、ただのガイドだと思います。あなたのコードブロックはそのままREADABLEとMAINTAINABLEです。これは簡単ですが、実際に変更したい場合は、以下の2つのアプローチを試してみて、複雑さが低いかどうかを確認してください。

注：私は今コンパイラを持っていないので、それについては事前に。

最初のアプローチ：

Map<String, String> multipliers = new HashMap<String, Float>(); 
    map.put("country", country); 
    map.put("exchange", exchange); 
    map.put("ccp", ccp); 
    map.put("tenant", tenant); 

その後、我々はただ

return map.get(type) == null ? name : map.get(type).getName() + HOLIDAY_CALENDAR; 

右の要素をつかむために第二のアプローチをマップを使用することができます。

あなたのすべてのオブジェクトは、同じメソッドを持っているので、あなたのことができInterfaceをgetName()メソッドに追加し、メソッド署名を次のように変更します。

getCalendarName(YourInterface yourObject){ 
    return yourObject == null ? name : yourObject.getName() + HOLIDAY_CALENDAR; 
} 

Answer 3

スイッチ比較の前に、すべてのNULL比較を削除して確認することができます。その場合、複雑さは4分の1かそれ以上になります。

Answer 4

オブジェクト：国、cpp、交換所、テナントが同じインターフェースを共有している場合。 ObjectWithGetName次のようにコードをリファクタリングできます

public String getCalenderName() { 
    ObjectWithGetNameMethod calendarType = null; 
    switch (type) { 
     case COUNTRY: 
      calendarType = country; 
      break; 
     case CCP: 
      calendarType = cpp; 
      break; 
     case EXCHANGE: 
      calendarType = exchange; 
      break; 
     case TENANT: 
      calendarType = tenant; 
      break; 
     default: 
      calendarType = null; 
    } 
    return (calendarType != null ? (calendarType.getName() + HOLIDAY_CALENDAR) : name); 
    } 

はまた、私はそれがさまざまな場所で使用される何かの魔女のように見えるので、別の方法にスイッチを動かしていいだろうと思います。

Answer 5

public String getName() { 
    if (type == null) { 
     return name; 
    } 
    if (type == BusinessCalendarType.COUNTRY) { 
     return country == null ? name : country.getName() + HOLIDAY_CALENDAR; 
    } else if (type == BusinessCalendarType.CCP) { 
     return ccp == null ? name : ccp.getName() + " CCP" + HOLIDAY_CALENDAR; 
    } else if (type == BusinessCalendarType.EXCHANGE) { 
     return exchange == null ? name : exchange.getName() + HOLIDAY_CALENDAR; 
    } else if (type == BusinessCalendarType.TENANT) { 
     return tenant == null ? name : tenant.getName() + HOLIDAY_CALENDAR; 
    } else { 
     return name; 
    } 
} 

これはあなたの最初の目的は唯一の循環的複雑度を軽減することであるならば、あなたは以下のように、名前を取得する各方法のためのメソッドを作成する必要があります私

Answer 6

のために働きました。

public String getCalenderName() { 
    switch (type) { 
    case COUNTRY: 
     return getCountryName(); 
    case CCP: 
     return getCcpName(); 
    case EXCHANGE: 
     return getExchangeName(); 
    case TENANT: 
     return getTenantName(); 
    default: 
     return name; 
    } 
} 

private String getCountryName() { 
    return country == null ? name : country.getName() + HOLIDAY_CALENDAR; 
} 

private String getCcpName() { 
    return ccp == null ? name : ccp.getName() + " CCP" + HOLIDAY_CALENDAR; 
} 

private String getExchangeName() { 
    return exchange == null ? name : getName.toString() + HOLIDAY_CALENDAR; 
} 

private String getTenantName() { 
    return tenant == null ? name : getName.toString() + HOLIDAY_CALENDAR; 
} 

---

注あなたの具体的な例では、私はあなたが（少なくとも）集まる1クラス4は非常に類似した行動を持っていると仮定します。リファクタリングは、例えば、1つの基本実装（抽象的であるかどうか）と4つの他の継承クラスを持つためには、確かに意味をなさないでしょう。