人力検索はてな
モバイル版を表示しています。PC版はこちら
i-mobile

以下のソースの場合、どれが書き方としてすっきりすると思いますか?順位と理由をお答えください。
(仕様によって違うよーと言うのであればその理由も御願いします。)

※順位の書き方は
3 > 1 > 2
と書いてください。
(3番が一番優れていると思う場合)


1)
public String run(String item) {
if (item == null) {
return null;
}

//なんか処理***

return 結果;
}
//**************************************

2)
public String run(String item) {

String 結果 = null;
if (item != null) {

//なんか処理***
}

return 結果;
}
//**************************************

3)
public String run(String item) {
if (item == null) {
throw new NullPointerException("Item is Null.");
}

//なんか処理***

return 結果;
}
//**************************************

●質問者: あーる
●カテゴリ:コンピュータ 学習・教育
✍キーワード:String ソース 仕様
○ 状態 :終了
└ 回答数 : 8/8件

▽最新の回答へ

[1]2>1>3 taknt

ぱっとみた感じで。


[2]2>3>1 gajin777

戻り値が何なのか追いやすい


[3]仕様により変わる dev_zer0

1と3は

// なんか処理***

でitemにnullが入ることが仕様上ありえるのか、ありえないのかで変わると思われます。

itemにnullが入ることが仕様上存在し得る場合、例外をなげるべきではありませんし、

itemにnullが入ることが仕様上あり得ない場合、そのままリターンすべきではありません。


2は論外です。構造化プログラミングでは入口、出口が1つであるべきいう下りがありますが

今は1つの引数チェックだけですが、複数の引数チェックを行うとブロックが深くなりすぎて見にくくなります。

構造化プログラミングは原則であり、完全に適用すると逆に見づらくなる典型的な例です。


よって、私は

itemにnullが入ることが仕様上存在し得る場合

1 > 2 > 3

# 3は仕様上存在し得るにも関わらず、呼び元でtry/catchしなければいけないのが冗長です


itemにnullが入ることが仕様上存在し得ない場合

3 > 1 > 2

# 1, 2でもどこかで例外が発生するでしょうが、例外は出来るだけ早く検出すべきと考えます。


[4]>3 Re:仕様により変わる PaulYamamoto

String 結果 のスコープを狭くするという意味でも1 > 2だと思います。


3は論外です。IllegalArgumetExceptionなら分かるけど、

NullPointerExceptionはnullオブジェクトのpropertieを参照したとき用なので。


あと、例外と、値返しは基本的に意味が違うので1,2と3は比較になりません。

結果がnullを返してはまずい場合は、3だけが正解でそれ以外はバグです。


結果がnullを返しても良い場合 & itemにnullを代入しても良い場合は

1,2が正解で3はバグです。


それと、ver1.4以降なら

public String run(String item) {

assert item != null : "itemがnullでは困ります!";

//なんか処理

return 結果;

}

という書き方もあります。


//なんか処理

をどうしても実行してもらいたい場合はこのほうがお薦めです。


[5]>4 Re:仕様により変わる aiaru

IllegalArgumentExceptionですか。なるほど!!

でも、引数が null と引数の値が想定外と分けたい場合は

NullPointerException を使う場合もあるので

論外は言いすぎな気がしますね。

例えば

java.lang.System.getProperty(String key, String def)

は、

key が null の場合は NullPointerException

key が 空 の場合は IllegalArgumentException

となっています。


でも、説明としては

IllegalArgumentException

の方が客観的にわかりやすいと言うのは、もっともだと思います。

そうすると、前提条件として引数をチェックしているというのがソースから読み取れますものね。

考慮が足らず、すみませんでした。 m(__)m


[6]>3 同意 yuki_n

3に関しては、他と仕様が違うので比較できない。

1>2

3は別次元

dev_zer0様の意見に少し追加。(個人的意見ですが)

『なんか処理』が長くなると(100ステップとか)、

if文の開始と終了がどこなのか、判別し辛くなる。

まぁ、1関数内で100ステップとかなってくると、

また別の問題もあるかもしれませんが。。例えということで。


[7]3>1>2 john-frank

3>1>2 です。

どれが一番すっきりするか?との問いですが、すっきりがよくわかりませんでしたので、アプリを作るのに一番ふさわしいという観点で選びました。

2は一番避けたいなぁと思いました。変数を初期化したはいいものの、結果になんか処理結果がちゃんと入っていなかったというバグたくさん見ました。前の方もおっしゃっていましたが、変数のスコープは可読性を損なわない限り全力で狭くするべきだと考えます。

残った3と1は、仕様によるので比較の対象にならないというのが正解だと思いますがあえて選択してみました。自分の経験を振り返ってみると3と1で迷うときは仕様が明確でないときです。仕様が明確でないのにコードを書くな!がルールですが、なかなかきれい事が通じない場面もあります。例外の種類はともかく、仕様が明確でない場合はNullであることを派手に警告すべきです。Nullを返すだけでは簡単に握り潰されてしまいそうです。


[8]1=3>2 yo-kun

1と3は仕様により異なります。

メソッドの仕様がそもそも異なるので当然ですよね。

どちらかといえば引数がStringであることを考慮するとStringにnullを許容しなければならないことは多くないと思われるので、(投げる例外の種類はともかく)例外を投げる3を推します。

例外の種類はdev_zer0さんが述べているようにIllegalArgumentExceptionが適当かと思います。(論外、とまでは思いませんが)


で、1と2ですが2よりも1を推す理由は2つあります。

最初の2行を見るだけで、以降のソースを見ることなく「引数がnullの場合はnullを返す」という仕様がはっきりと分かります。

2だとそうはいきません。もしかしたらelse?があるかもしれませんから。

個人的な好みですが、1と比較して2だとネストのレベルが1つ上がります。1でなら回避できるのに2のコードでネスト(というかインデント)がムダに増えるのが個人的には嫌です。

関連質問


●質問をもっと探す●



0.人力検索はてなトップ
8.このページを友達に紹介
9.このページの先頭へ
対応機種一覧
お問い合わせ
ヘルプ/お知らせ
ログイン
無料ユーザー登録
はてなトップ