Create STEP3.md

[achotto]

今回の問題
1. Two Sum

次の問題
349. Intersection of two arrays

[nodchip]

レビューワーがレビューをするにあたり、有用な情報となるため、どののソースコードを読んだか、リンクと、そのコードを読んだ時に感じたこと、考えたことなどを一緒に書いていただけるとありがたいです。

[achotto]

次に問題を解くときには、レビューや自分で見返したときのために、記録を残しておくようにしようと思います。スタイルガイドについても、ありがとうございます。

[nodchip]

参考までにスタイルガイドへのリンクを貼ります。

https://google.github.io/styleguide/javaguide.html#s4.6.2-horizontal-whitespace

Separating any keyword, such as if, for or catch, from an open parenthesis (() that follows it on that line

上記のスタイルガイドは唯一絶対のルールではなく、複数あるスタイルガイドの一つに過ぎないということを念頭に置くことをお勧めします。また、所属するチームにより何が良いとされているかは変わります。自分の中で良い書き方の基準を持ちつつ、チームの平均的な書き方で書くことをお勧めいたします。

[nodchip]

{ の前にスペースを空けることをおすすめします。

https://google.github.io/styleguide/javaguide.html#s4.6.2-horizontal-whitespace

Before any open curly brace ({), with two exceptions:
@SomeAnnotation({a, b}) (no space is used)
String[][] x = {{"foo"}}; (no space is required between {{, by item 10 below)

[oda]

= まで回すと配列外アクセスの可能性があるのでは。

[achotto]

おっしゃる通りですね。。lengthは要素数を返すので、=まで行くと、配列の最後のインデックスより大きくなってしまいますね。<=ではなく<にする必要がありました。ありがとうございます。

[naoto-iwase]

デバッグをしやすくするために、実際の値を含めるとより親切になると思います。

throw new IllegalArgumentException("No valid pair whose sum equals target: " + target);

[achotto]

たしかに、エラーメッセージに値を入れるほうが、よりわかりやすくデバッグがしやすくなりますね。
ありがとうございます。

[yas-2023]

お疲れ様です、
RuntimeException ですが、意味が広すぎるため、意図がコードから伝わりにくいかもしれません。
今回のケースでは、NoSuchElementException のような文脈にあった例外を選ぶと、読み手にとっても目的が明確になるように思いました。

if (nums == null || nums.length < 2) {
  throw new IllegalArgumentException("Input array must have at least two elements");
}

throw new NoSuchElementException("No valid pair found");

https://docs.oracle.com/javase/8/docs/api/java/lang/RuntimeException.html
https://docs.oracle.com/javase/8/docs/api/java/lang/IllegalArgumentException.html
https://docs.oracle.com/javase/8/docs/api/java/util/NoSuchElementException.html

java.lang.Object
└── java.lang.Throwable
├── java.lang.Error // JVMやシステムレベルのエラー
└── java.lang.Exception
├── java.lang.RuntimeException // 非検査例外 (unchecked)
│ ├── java.lang.IllegalArgumentException
│ ├── java.lang.NoSuchElementException
│ ├── java.lang.NullPointerException
│ ├── java.lang.IndexOutOfBoundsException
│ ├── java.lang.ArithmeticException
│ └── ...（その他 Runtime 系例外）
├── java.io.IOException // 検査例外 (checked)
├── java.text.ParseException
└── ...（その他 checked 例外）

[achotto]

返信が遅くなり失礼いたしました。お恥ずかしながらExceptionに種類があることが頭になかったので、調べて出てきたRuntimeExceptionを使用しましたが、文脈に合わせて例外を選択できるようにしたいと思います。
ありがとうございます。

[t9a-dev]

Suggested change

	for(int i = 0; i <= nums.length; i++){
	int complement = target - nums[i];
	if(checked_numbers.containsKey(complement)){
	return new int[]{checked_numbers.get(complement), i};
	}
	checked_numbers.put(nums[i], i);
	}
	int i = 0;
	for(int num : nums){
	int complement = target - num;
	if(checked_numbers.containsKey(complement)){
	return new int[]{checked_numbers.get(complement), i};
	}
	checked_numbers.put(num, i);
	i++;
	}

好みの問題ですが配列の境界を考えるのが好きではないので、自分ならこんな感じの書き方をするかなと思いました。
LeetCode上で実行できることを確認済ですがJavaの読み書きができないのでスタイルガイドや文化的に間違っているかもしれないです。

[achotto]

返信が遅くなり失礼いたしました。
拡張for文と言われる書き方なのですね。だいぶ前に習ったのですが、すっかり忘れていました。
配列の範囲をすでに間違って書いてしまったので、こちらのほうが確実でよい書き方かもしれないと思いました。javaスタイルガイドを確認しましたが、forループを書く時に拡張for文を使用してはいけないといったような記載は見当たらなかったです。拡張for文が選択肢として出てくるよう意識したいと思います。
ありがとうございます。

[kt-from-j]

ArrayListのindexOf()など配列等から何らかのインデックスを探すメソッドにおいても対象が見つからなかった場合、-1が返される※のでそれに則って-1を返そうという話ですね。
呼び出し側は、-1や0未満が返ってきたら例外処理を書くというのが定石になると思います。

※この挙動はJavaに限らず多くの言語でそうなっているように思います。

[achotto]

返信が遅くなり失礼いたしました。返すものがなかったとき-1を返す挙動があるのですね。勉強になりました。勉強のためjavaのドキュメントも確認してみようと思います。
ありがとうございます。