Conversation
|
|
||
| 入力配列に重複要素があった時に,何番目のindexを返すのか([cf](https://discord.com/channels/1084280443945353267/1227464441235509308/1229628764443643935)). | ||
| 今のコードだと左端とは限らないね.種類としてはまず2つに大別できる: | ||
| - とりあえずtargetと同じ値があるか探す |
There was a problem hiding this comment.
入力配列に重複要素があった時に、何番目のindexを返すのか
の実装方法の種類の一つですから、
「targetと等しい要素が複数あった場合は、それらのインデックスのうちのどれか1つを返す。」
の方が意図を表現できていると思いますね
There was a problem hiding this comment.
仰る通りですね,日本語としての対応がおかしい文章になってしまいました.
| --- | ||
|
|
||
| [区間は何を意味するか](https://github.com/seal-azarashi/leetcode/pull/38#discussion_r1836463140). | ||
| - `first`: `nums[first - 1]`より手前には`target`より小さいものしかない. |
There was a problem hiding this comment.
言いたいことは察せるんですが、
first = 0 , last = nums.size()
で初期化しているので、
nums[fist -1], nums[last]
と表記するのに違和感を持ちました。初期化時の値をそれぞれ代入すると
nums[-1] , nums[nums.size()]
と配列外参照に見えます。
first未満のインデックスの全要素はtargetより小さい、last以上のインデックスの全要素はtargetより大きい、などと自分なら書きます。
(first,lastの初期化時と更新時ともにこれらが不変条件となっているのが確認できると思います
There was a problem hiding this comment.
なんなら私の表現は間違っていますね.正しくは仰る通り
firstより手前のindexの要素はtargetより小さい
か,元の私の書き方に揃えるとしても
nums[first]より手前にはtargetより小さいものしかない.
ですしfirst - 1で言うとしても
nums[first - 1]とそれより手前にはtargetより小さいものしかない.
のようにinclusiveにする必要がありました.
| center = (left + right) / 2; | ||
| } | ||
|
|
||
| return center; |
There was a problem hiding this comment.
不変条件で考えるとcenterよりかは left, right どちらかを返す方が自然に思います.(実際はwhileループを抜けた段階で right = left なので,それらを使って center = (left+right) / 2 とした center もまた left = right = center になるんですが.)
| ``` | ||
|
|
||
| # Step 2 | ||
| どうでもいいけど,Javaの`Arrays.binarySearch`ってターゲットと同じ数値が見つからなかったらビット反転したものが返ってくるんだ([cf](https://docs.oracle.com/javase/jp/8/docs/api/java/util/Arrays.html#binarySearch-byte:A-byte-)). |
There was a problem hiding this comment.
ビット反転と言うよりかは,targetが見つからなかった場合は負の,見つかった場合は正の,インデックスを返しているんだと思います.
で,すべての要素よりもtarget が小さかった場合とtargetが0番目の要素と等しかった場合の区別をしたくて, - 1 をつけているんだと思います.つけないと,両者の結果が
There was a problem hiding this comment.
で,すべての要素よりもtarget が小さかった場合とtargetが0番目の要素と等しかった場合の区別をしたくて,
-1をつけているんだと思います.つけないと,両者の結果が$\pm 0= 0$ で等しくなって区別できないので.
この発想なかったです.ありがとうございます.
それでついた-1によって自然に2の補数と同じ計算になるんですね.
| continue; | ||
| } else { | ||
| last = mid; | ||
| } |
There was a problem hiding this comment.
136行目のcontinue は不要ですね.
if (nums[mid] < target){
first = mid + 1;
continue;
}
last = mid;あるいは
if (nums[mid] < target){
first = mid + 1;
} else {
last = mid;
}のどちらかで良いと思います.
There was a problem hiding this comment.
Code2については,else用いていないため,2つ目のifを通っても3つ目のifを通る可能性が(このコードではありえないですが,一般には)ある.この時,3つ目のifを考える時に,厳密に場合分けするなら2つ目のifを通ったかどうかを頭に入れておかねばならなくなってこんがらがる.そこでcontinueを入れることで,「3つ目を考えるときは一旦2つ目の条件は忘れていいよ」と言う意図で入れてあります.
言い方を変えると,これらのif文を場合分けというより,フィルターのようなものと捉えているとも言えます.「ベルトコンベアでネジが運ばれてきて,色の検査,重さの検査,形の検査とあって,どれか引っ掛かったら即別ルートに分岐,以降の検査はもうしない」みたいな感じです
一方,今回のロジックでは場合分けの方が素直で,逆にこのフィルター的な書き方は素直でなく,読み手に無駄な疑問を抱かせてしまう恐れがあると気づきました.ありがとうございます.
Code4は,仰る通りelseがあるのでcontinue;は不自然でした.どうせ場合分けなので.
There was a problem hiding this comment.
2つ目のifと3つ目のifの処理の片方のみが実行されることを保証したいなら,
if (nums[mid] == target) {
return mid;
}
if (nums[mid] < target) { // 2つ目のif
first = mid + 1;
} else if (nums[mid] > target) { // 3つ目のif
last = mid;
} else {
hoge(); // =, >, < のいずれも成り立たない一般の場合についての何かしらの処理
}
で事足りるように思いますね. (更に付け加えるなら,<, > のどちらも成り立ちうる一般の場合を考慮するなら,逆に, =, <, > のいずれも成り立たない場合も考慮したいなと思います.)
他のif 文のブロックの分量とかのトレードオフで if 文内でcontinue して見やすくすることはよく見るのですが,今回の場合だとそれぞれのブロックに分量差が殆どないので,あえて else if を使わずにcotinue 連打する必要性はうすく思います.
(ところで,2つめのif は else if にするかどうかは(1つ目の if がreturn していてその他のif 文の中身の処理と合わせて実行されないことが保証できるので)好みの問題だと思います.)
There was a problem hiding this comment.
読み手に無駄な疑問
まさにこれで, あえて else if を使わずに continue; 連打した理由ってなんだろうな〜〜って思いました.
There was a problem hiding this comment.
更に付け加えるなら,<, > のどちらも成り立ちうる一般の場合を考慮するなら,逆に, =, <, > のいずれも成り立たない場合も考慮したいなと思います.
よく考えたらフィルターとして使うなら基本的にこの視点があるべきですね.ベルトコンベアの先で何か統一的な処理をするために検査をかけて例外を取り除く分けですし.
There was a problem hiding this comment.
arahi10さんのおっしゃる通り、else ifのブロックがすごく長いときやelse ifが延々と続く時などは、下になんか書いているのかなって見にいく手間を減らすために、continueする場合がありますね。
Code 2の場合だと、mutually exclusive, collectively exhaustiveなことを明示するために、else if / elseで繋げた方が良さそうです。
| return searchInsertHelper(nums, target, mid + 1, last); | ||
| } else { | ||
| return searchInsertHelper(nums, target, first, mid); | ||
| } |
There was a problem hiding this comment.
好みの問題ですが,上の if で return している場合は else を書かないほうが好きです.
if (nums[mid] < target) {
return searchInsertHelper(nums, target, mid + 1, last);
}
return searchInsertHelper(nums, target, first, mid);There was a problem hiding this comment.
個人的には、処理が対称となっているため、 else を書いて対称っぽく見せるのが好きです。趣味の範囲だと思います。
| while (right - left > 0) { | ||
| if (nums[center] == target) { return center }; | ||
| if (nums[center] > target) { right = center }; | ||
| if (nums[center] < target) { left = center + 1 }; |
There was a problem hiding this comment.
あまり if (~~~) {~~~} と1行で済ませてしまう書き方は見ないと思うのと, セミコロンの位置が3行ともおかしく見えます.
There was a problem hiding this comment.
仰る通りですね,コンパイルエラーになります.
if (~~~) {}は見ない
こちらは,書くとしたら見るのは
if (nums[center] == target) return center;か,あるいは
if (nums[center] == target) {
return center;
}の2つと言う認識であっていますでしょうか.
There was a problem hiding this comment.
はい,そのどちらかか,1つ目に改行を加えたもの;
if (nums[center] == target)
return center;でしょう.改行しつつ波括弧つけるもの;
if (nums[center] == target) {
return center;
} が一番安心でしょうね.(好みとしては,これに統一したいです)
ところで,2つ目の例の最後のセミコロンはいらないと思います.
There was a problem hiding this comment.
ぶら下がり if 文はバグの原因になる場合があるため、原則避けたほうが無難だと思います。詳しくは過去のコメントをご参照ください。
| if (nums[mid] == target) { | ||
| return mid; | ||
| } | ||
| if (nums[mid] < target) { |
There was a problem hiding this comment.
if 文 2 個が両方とも conitnue で終わっているため、 continue をする意味が薄いように感じました。自分なら continue せず以下のように書くと思います。
if (nums[mid] < target) {
first = mid + 1;
} else {
last = mid;
}- nums[mid] == target
- nums[mid] < target
- nums[mid] > target
が並列に並んでいないのがやや気持ち悪いのですが、許容範囲かなと思います。
| return searchInsertHelper(nums, target, mid + 1, last); | ||
| } else { | ||
| return searchInsertHelper(nums, target, first, mid); | ||
| } |
There was a problem hiding this comment.
個人的には、処理が対称となっているため、 else を書いて対称っぽく見せるのが好きです。趣味の範囲だと思います。
| while (right - left > 0) { | ||
| if (nums[center] == target) { return center }; | ||
| if (nums[center] > target) { right = center }; | ||
| if (nums[center] < target) { left = center + 1 }; |
There was a problem hiding this comment.
ぶら下がり if 文はバグの原因になる場合があるため、原則避けたほうが無難だと思います。詳しくは過去のコメントをご参照ください。
|
他の2パターンも書いてみると学びが深まると思いました。 |
35. Search Insert Position