現在Javaでif文のコードを書いています。
if文のコードの良い書き方(リファクタリング)について教えて頂きたいです。
次のコードはどのように書くのがベターでしょうか?
Integer型の変数num1,num2があり、どちらもnullでない場合、差を計算し、その結果を返すメソッドです。
私は以下のように書きましたが、もっといい書き方があれば教えて頂きたいです。
if文のreturnは早く返した方がいいとリファクタリングの本で見た気がするのでその観点で実装しています。
public int getNum3(Integer num1,Integer num2){
int num3 =0;
if(num1==null&&num2==null){
return num3;
}
num3 = num2-num1;
return num3 ;
}
気になる質問をクリップする
クリップした質問は、後からいつでもMYページで確認できます。
またクリップした質問に回答があった際、通知やメールを受け取ることができます。
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
回答10件
0
ベストアンサー
私ならこう書きます。
Java
1 public int getNum3(Integer num1, Integer num2) { 2 if (num1 == null) return 0; 3 if (num2 == null) return 0; 4 return num2 - num1; 5 }
投稿2017/05/17 14:35
総合スコア6299
0
回答がもう出てますので勝手ながら比較部分のコードの指摘を。。。
int型は符号付き数値型です(2147483647 ~ -2147483648)
符号付き数値型の場合は、その最大の差をその型では表現できません。
例)2つのInteger型の最大の差は
上限値 | Integer.MAX_VALUE |
下限値 | Integer.MIN_VALUE |
の時で値は4294967295になります。そのため比較部分と戻り値の型はlongになります。
Java
1 public long getNum3(Integer num1, Integer num2) { 2 if (num1 == null || num2 == null) { 3 return 0; 4 } 5 return (long)num2 - num1; 6 }
getNum3(Integer.MIN_VALUE, Integer.MAX_VALUE); をこの値を引数に渡して見るとと分かりやすいかもーです。
投稿2017/05/17 14:27
編集2017/05/17 14:39総合スコア5846
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
0
Java8のOptionalを使うとこんな感じ
java
1public int getNum3(Integer num1,Integer num2){ 2 Optional<Integer> i1 = Optional.ofNullable(num1), 3 i2 = Optional.ofNullable(num2); 4 return i1.flatMap(x -> i2.map(y -> y - x)).orElse(0); 5}
異常を伝えるためにもOptionalで返すべきですね。
java
1public Optional<Integer> getNum3(Integer num1,Integer num2){ 2 Optional<Integer> i1 = Optional.ofNullable(num1), 3 i2 = Optional.ofNullable(num2); 4 return i1.flatMap(x -> i2.map(y -> y - x)); 5}
投稿2017/05/17 14:17
編集2017/05/18 06:09総合スコア20651
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
0
num1==null&&num2==null
ではなく num1==null || num2==null
ですよね?書き方は普通だと思います。
私なら、先にリターンする方は return num3;
ではなく、直接return 0;
と書きますが。
投稿2017/05/17 13:33
総合スコア936
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
0
私ならif文でなく↓の一行で戻り値返しします。
return (NULL != num1 && NULL != num2) ? 0 : num2 - num1;
投稿2017/05/17 13:49
編集2017/05/17 13:50総合スコア4830
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2017/05/17 13:58
2017/05/17 14:11 編集
0
いろんな意見があり面白いですね。
10件目くらいの意見としては、リファクタリングがコードを易くする作業だとするならメソッドの定義に違和感をおぼえます。
引き算をするメソッドがgetNum3と言う点のツッコミをほっといても、引き算をするメソッドでNullを許容している点はおかしいと思います。さらに、その時の戻り値が0これって以下のケースですべて戻り値が0になると言うことです。
getNum3(1000, 1000) == 0;
getNum3(0, 0) == 0;
getNum3(Null, 84) == 0;
getNum3(-553, Null) == 0;
どういう要件か解りませんがこれが正しい動作である気がしません。
そのまま例外を投げるか、Null伝播が適当だと思われます。
getNum3(1000, 1000) == 0;
getNum3(0, 0) == 0;
getNum3(Null, 84) == Null;
getNum3(-553, Null) == Null;
投稿2017/05/18 04:55
総合スコア2883
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2017/05/18 05:03
2017/05/18 05:13
2017/05/18 08:57
2017/05/18 10:31 編集
2017/05/18 12:04
0
koko_uさんが既にご指摘されていますが、パラメタのnull判定をしてどちらかがnullだった場合の
Java
1return num3;
はちょっとどうかなと思います。return 0のほうがベターかなと。
例えばエラーを示す値が9だったとして、その場合、このような方針で似たような関数のリファクタリングをする際に、以下のようなコードにはしないと思います。
Java
1public int getNum3(Integer num1,Integer num2){ 2 int num3 =9; //注目!! 3 if(num1==null&&num2==null){ 4 return num3; 5 } 6 num3 = num2-num1; 7 return num3 ; 8}
Java
1public int getNum3(Integer num1,Integer num2){ 2 int num3 =0; 3 if(num1==null&&num2==null){ 4 num3 = 9; //注目!! 5 return num3; 6 } 7 num3 = num2-num1; 8 return num3 ; 9}
可読性が落ちますし、後者は処理も無駄に増えてしまっています。
エラーや、何も処理しなかったことを示す値が明確に決まっている場合は、その値を直接returnするほうがベターだと思います。
Java
1 2//getNum3 3//パラメタ2-パラメタ1の差を返却する 4//パラメタのどちらかがnullの場合は0を返却する 5public int getNum3(Integer num1,Integer num2){ 6 int num3; 7 if(num1==null&&num2==null){ 8 return 0; 9 } 10 num3 = num2-num1; 11 return num3 ; 12}
あとは、これは投稿用に適当につけた名前だと思いますので本質的な指摘ではないのですが、可読性の観点からはもう少し変数名や関数名ににこだわると良いのではないでしょうか。
Java
1 2//getNumDifference 3//パラメタ2-パラメタ1の差を返却する 4//パラメタのどちらかがnullの場合は0を返却する 5public int getNumDifference(Integer subtrahend,Integer minuend){ 6 int difference; 7 if(subtrahend== null || minuend== null){ 8 return 0; 9 } 10 difference = minuend - subtrahend; 11 return difference; 12}
投稿2017/05/17 14:27
編集2017/05/18 06:11総合スコア1947
0
では毛色の違ったこんなのはどうでしょうか。
Java
1public int getNum3(Integer num1, Integer num2) { 2 return (getNum(num1) - getNum(num2)); 3} 4 5public int getNum(Integer n) { 6 return (n != null) n : 0; 7}
--- 追記 ---
調べたらこんなんでいけそう
Java
1public int getNum3(Integer num1, Integer num2) { 2 return (ObjectUtils.defaultIfNull(num1, 0) - ObjectUtils.defaultIfNull(num2, 0)); 3}
--- 追記 ---
ということで、他の方のを参考にしてください。(^_^;
投稿2017/05/17 14:05
編集2017/05/17 14:19総合スコア7458
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2017/05/17 14:14
2017/05/17 14:16
2017/05/17 14:33
2017/05/17 14:42
0
Javaは書いたことないですが。。。
java
1public int getNum3(Integer num1, Integer num2){ 2 3 4 if(num1 == null || num2 == null) { 5 return 0; 6 } 7 8 return num2 - num1; 9 10}
java
1public int getNum3(Integer num1, Integer num2){ 2 3 4 return num1 == null || num2 == null ? 0 : num2 - num1; 5 6 7}
投稿2017/05/17 13:33
編集2017/05/17 14:08退会済みユーザー
総合スコア0
あなたの回答
tips
太字
斜体
打ち消し線
見出し
引用テキストの挿入
コードの挿入
リンクの挿入
リストの挿入
番号リストの挿入
表の挿入
水平線の挿入
プレビュー
質問の解決につながる回答をしましょう。 サンプルコードなど、より具体的な説明があると質問者の理解の助けになります。 また、読む側のことを考えた、分かりやすい文章を心がけましょう。
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2017/05/17 14:57
2017/05/17 15:10
2017/05/18 04:29