Skip to content

278. First Bad Version#83

Open
kitano-kazuki wants to merge 1 commit into
mainfrom
278-first-bad-version
Open

278. First Bad Version#83
kitano-kazuki wants to merge 1 commit into
mainfrom
278-first-bad-version

Conversation

@kitano-kazuki
Copy link
Copy Markdown
Owner

Comment thread memo.md
if isBadVersion(1):
return 1

good_left = 1
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good_leftというネーミングですが、goodの左端?ということを想起させる可能性があり、若干わかりにくい気がしました。bad_rightについても同様です。

二分探索として読むときに読みやすいか?ということはありますが、(last_)good_version と(first_)bad_versionがの方が意味的には正しいと思います。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good_leftというネーミングですが、goodの左端?ということを想起させる可能性があり、若干わかりにくい気がしました。

自分も同じように思いながら書いていました。

(last_)good_version と(first_)bad_versionの方が読みやすくて良さそうです

Comment thread memo.md
* https://github.com/rihib/leetcode/pull/33
* > isBadVersion 次第ですが、これだとループ一回で最大3回呼ばれるのが少し気になりますね。場合によっては10分くらいかかるとても重い処理かもしれません
* 最初の判定で2回呼んでいるのが無駄かも??
* でも結局最初や最後にbad versionがあると, 最初に判定しない限り工数が増えすぎちゃうよな
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この問題文だとbadが存在しているという前提にも見えるので、その前提であれば if not isBadVersion(n): の方は不要だと思います。
一般論として最後まで正しい可能性もあるので、一概にNGということではないです。
ループの中で呼ぶよりは問題ないと思います。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この部分は、badが存在している前提をとっぱらった時にどうするかで迷っていました。

自分もループの中で呼ぶよりは問題ないし、最初に呼ぶのはしょうがない処理なのかなとは思いました。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

badが1-nのバージョンに存在するのがレアな状況では、大多数のケースでAPI呼び出しの回数を大幅に削減できるのは、個人的には便利でいいなと思いました。

-1を返すべきかはケースバイケースかと思いますが、今回所与のsignatureにおいては良さげに感じました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants