-
Notifications
You must be signed in to change notification settings - Fork 0
0001. two sum #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
def twoSum(self, nums: List[int], target: int) -> List[int]: | ||
pair_index = {} | ||
for i, num in enumerate(nums): | ||
if target - num in pair_index: | ||
return [i, pair_index[target - num]] | ||
pair_index[num] = i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この関数の処理、return 文まで到達しない可能性がありますね。Leetcode の問題文には、そのようなことになる値が引数として渡されないとありますが、その旨が関数の実装にも反映されていた方がいいように思います。
以前自分が Java で実装した際、return 文まで到達しなかった際に通る関数の最下部にて exception を throw するようにしてみましたが、Python で同等の処理を書いておくといったことをしてもいいのかなと思いました (参考)。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
なかった場合、何が返るかは答えられますか。実行して確認してみましょう。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます。なかった場合は空のListの[]が返却されますね。。
問題文の前提上は必ずreturnされるとしても、確かにこの関数だけを見るとreturnされない可能性があるのでは?という迷いが生じますね。
そういった意味で例外処理をするなどで関数内で最後まで到達することがあり得ない実装にしておいた方がよさそうですね・・勉強になります!
上記を書き直して実装。 | ||
|
||
思考ログ | ||
- numsとnumが似ているのは混同する可能性があるので他の命名にした方が良さそう? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
自分はnum
-> n
でもこの場合は十分伝わりそうです
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます!n
はnumberを表すのは自明ですもんね、、
かつ、num
とnums
を混同する可能性があるという点で、n
の方がより良いように感じますね。。
|
||
思考ログ | ||
- numsとnumが似ているのは混同する可能性があるので他の命名にした方が良さそう? | ||
- pair_indexは分かりやすい名前なのか? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
個人的にはpair_indexだとわかりにくいかなと思います。Tupleやpairを想起させられるので
例えば、num_to_index
にして、complement = target - num
に一度バインドした後で
if complement in num_to_index
にすると意図が伝わりやすいかなと思いました
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます。別コメントにもありましたが、確かにpair_indexだとindexのペアが格納されているように感じますね、、
参考例もありがとうございます。あくまで数値とindexを持っていることが分かるような変数名にしたいと思います!
- ```「プログラムの実行に必要なメモリの量が入力に対してどのように変化するか」という指標を空間計算量といいます。``` | ||
- 2次元配列はO(n^2)、1次元配列はO(n)、変数の格納のみの場合はO(1)といった計算量になる | ||
- https://atcoder.jp/contests/apg4b/tasks/APG4b_w?lang=ja | ||
- 時間計算量と空間計算量はLeetCodeの```submit```後の実行結果から確認できるので答え合わせに良さそう。(```Runtime```や```Memory```の```Analyze Complexity```)※プレミアムに加入が必要なのか・・・ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
というよりもレビューに出すときに一緒に書いておけば、間違っている場合誰かが指摘してくれると思います
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
その方が良さそうですね、、自分で考えて記載するようにします!
# 問題概要 | ||
問題: 1. Two Sum | ||
|
||
https://leetcode.com/problems/two-sum/description/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
問題のリンクはプルリクのコメントに記載していただけた方がありがたいです。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます。反映しました!
```python | ||
class Solution: | ||
def twoSum(self, nums: List[int], target: int) -> List[int]: | ||
pair_index = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
自分がPythonに慣れていないことが原因ではあるのですが、最初ここを読んだ時、変数名もあってか、
pair_index = {(i0, j0), (i1, j1), ...}
のようにインデックスの組みを格納するset型の変数かと思ってしまいました。調べたら、空のsetを宣言するときは必ずset()を使うらしいので、Python慣れている人なら= {}
を見てすぐ辞書型とわかるかもしれませんが、dict()
の方がわかりやすいと思いました。
Pythonに慣れていない人が見たら、という視点なのでそんなこともないのかもしれません
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここら辺見てもらえると良いかと思います。dict()の場合は追加の関数呼び出しのオーバーヘッドがあるため、Pylintなどだと{}を使うようにメッセージが出るらしいです。
rihib/leetcode#8 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コメント&補足ありがとうございます。
pyhonって書き方が簡易的なので分かりにくさはありますよね・・・
dict()
と{}
の違いは全く意識したことがなかったですが、{}
の方が速くよく使われるのですね!
dict()を使った際は{}を使うようにメッセージが出るようになっているっぽい。
→これも全く知らなかったです…{}
の方がより速いという点で推奨されているようなので、{}
を採用するようにします!そのうえで変数名については分かりやすい命名になるよう工夫したいと思います。
- 有効な答えは必ず存在する | ||
- どのような解き方ができるか | ||
- 愚直に配列をループさせて和がtargetとなる要素のindexを返せばいけそう | ||
- 2つのループを用意する。iは0~n-1、jは1~nまで回す |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
時間計算量や実行時間は考慮しましたか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ご指摘ありがとうございます。
考慮できていなかったです。。solutionを比較する際に重要な指標になるかと思いますので、今後は時間計算量や実行時間も考慮して解答方法を考えたいと思います。
for i in range(len(nums)-1): | ||
for j in range(i+1, len(nums)): | ||
if nums[i] + nums[j] == target: | ||
ans = [i,j] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
,
の後にスペースを 1 つ入れることをお勧めします。
https://google.github.io/styleguide/pyguide.html#s3.6-whitespace
No whitespace before a comma, semicolon, or colon. Do use whitespace after a comma, semicolon, or colon, except at the end of the line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
上記リンク先について知らなかったです…
全く意識できていなかったので、今後は[i, j]
と記載したりStyle Guidesを意識した体裁にしていきたいと思います!
問題: 1. Two Sum
https://leetcode.com/problems/two-sum/description/
言語: Python