2017-12-29 47 views
3
const someFailedAction = (caseIds, growlMessages) => { 

    if (caseIds.length > 1) { 
     toastr.error(growlMessages[0], errorToastrOptions); 
    } else if (isCaseDetailsDisplayed) { 
     toastr.error(growlMessages[1], errorToastrOptions); 
    } else if (errorParts.fieldIds.length === 0) { 
     toastr.error(growlMessages[2], errorToastrOptions); 
    } else { 
     toastr.error(growlMessages[3], errorToastrOptions); 
    } 
} 

私は、上記のような一連の条件文をさまざまなケースアクションの失敗で実行します。ほとんどのアクションは同じif/elseIf構造体を持ちますが、いくつかはelseIfを追加したり、1つ以上の減算をしたりします。型を渡したり、いくつかの条件文を実行したり、オブジェクトを返すための良いデザインパターンは何でしょうか?

const SomeOtherFailedAction = (caseIds, growlMessages) => { 

    if (caseIds.length > 1) { 
     toastr.error(growlMessages[0], errorToastrOptions); 
    } else if (isCaseDetailsDisplayed) { 
     toastr.error(growlMessages[1], errorToastrOptions); 
    } else { 
     toastr.error(growlMessages[2], errorToastrOptions); 
    } 

} 

私は長い間、ネストされた、繰り返しswitch文をせずに、メッセージの種類と配列で渡すことができますのための優れたデザインパターンがあった場合、私は思っていました。

+0

をあなたの関数を呼び出す:' toastr.error(growlMessages [%インデックス%]、オプション)。 '' someFailedAction'の終わりに 'error'呼び出しを行うのは良くないでしょうが、' growlMessages'に 'index'を指定するそれぞれの条件で – Andy

+0

' isCaseDetailsDisplayed'と 'errorParts'はどこから来たのですか? – Bergi

答えて

4

ただ、boolean配列を使用してsucceeedsインデックス見つける:

const i = [ 
caseIds.length > 1, 
isCaseDetailsDisplayed, 
errorParts.fieldIds.length === 0, 
true 
].indexOf(true); 

toastr.error(growlMessages[i], errorToastrOptions) 

を代わりに、あなたは機能の再利用可能な配列を格納することができ、次々と実行し、関数がtrueを返した最初のインデックスを返します。

const tests = [ 
() => caseIds.length > 1, 
() => isCaseDetailsDisplayed, 
() => errorParts.fieldIds.length === 0, 
() => true 
] 

const i = tests.findIndex(f => f()); 

toastr.error(growlMessages[i], errorToastrOptions) 
+0

本当に良い解決策! +1 – Andy

+0

迅速な返信をありがとう。私はたくさんのコードをリファクタリングしていますが、ソリューションを見たときに、より良いソリューションを見出すことが難しい場合もあります(悪いですが) – kyle

3

ジョナソリューションは非常に効率的です。デフォルトのインデックスを設定することであなたが持っているものを消耗し、あなたの条件でのみ上書きすることができます。それから私は内部が `someFailedAction`機能は、各条件で、ほぼ同じラインに持っていることがわかり、あなたの場合は最後に一回

const someFailedAction = (caseIds, growlMessages) => { 

    let idx = 3// replaces final `else` and is default 

    if (caseIds.length > 1) { 
     idx=0;   
    } else if (isCaseDetailsDisplayed) { 
     idx = 1;   
    } else if (errorParts.fieldIds.length === 0) { 
     idx= 2;   
    }// last `else` removed since already have a default 

    toastr.error(growlMessages[idx], errorToastrOptions); 
} 
関連する問題