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.
この関数の処理、return 文まで到達しない可能性がありますね。Leetcode の問題文には、そのようなことになる値が引数として渡されないとありますが、その旨が関数の実装にも反映されていた方がいいように思います。
以前自分が Java で実装した際、return 文まで到達しなかった際に通る関数の最下部にて exception を throw するようにしてみましたが、Python で同等の処理を書いておくといったことをしてもいいのかなと思いました (参考)。
There was a problem hiding this comment.
ありがとうございます。なかった場合は空のListの[]が返却されますね。。
問題文の前提上は必ずreturnされるとしても、確かにこの関数だけを見るとreturnされない可能性があるのでは？という迷いが生じますね。
そういった意味で例外処理をするなどで関数内で最後まで到達することがあり得ない実装にしておいた方がよさそうですね・・勉強になります！
| 上記を書き直して実装。 | ||
|
|
||
| 思考ログ | ||
| - numsとnumが似ているのは混同する可能性があるので他の命名にした方が良さそう？ |
There was a problem hiding this comment.
ありがとうございます！nはnumberを表すのは自明ですもんね、、
かつ、numとnumsを混同する可能性があるという点で、nの方がより良いように感じますね。。
|
|
||
| 思考ログ | ||
| - numsとnumが似ているのは混同する可能性があるので他の命名にした方が良さそう？ | ||
| - pair_indexは分かりやすい名前なのか？ |
There was a problem hiding this comment.
個人的にはpair_indexだとわかりにくいかなと思います。Tupleやpairを想起させられるので
例えば、num_to_indexにして、complement = target - num に一度バインドした後で
if complement in num_to_indexにすると意図が伝わりやすいかなと思いました
There was a problem hiding this comment.
ありがとうございます。別コメントにもありましたが、確かに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.
というよりもレビューに出すときに一緒に書いておけば、間違っている場合誰かが指摘してくれると思います
There was a problem hiding this comment.
その方が良さそうですね、、自分で考えて記載するようにします！
| # 問題概要 | ||
| 問題: 1. Two Sum | ||
|
|
||
| https://leetcode.com/problems/two-sum/description/ |
| ```python | ||
| class Solution: | ||
| def twoSum(self, nums: List[int], target: int) -> List[int]: | ||
| pair_index = {} |
There was a problem hiding this comment.
自分がPythonに慣れていないことが原因ではあるのですが、最初ここを読んだ時、変数名もあってか、
pair_index = {(i0, j0), (i1, j1), ...}のようにインデックスの組みを格納するset型の変数かと思ってしまいました。調べたら、空のsetを宣言するときは必ずset()を使うらしいので、Python慣れている人なら= {}を見てすぐ辞書型とわかるかもしれませんが、dict()の方がわかりやすいと思いました。
Pythonに慣れていない人が見たら、という視点なのでそんなこともないのかもしれません
There was a problem hiding this comment.
ここら辺見てもらえると良いかと思います。dict()の場合は追加の関数呼び出しのオーバーヘッドがあるため、Pylintなどだと{}を使うようにメッセージが出るらしいです。
rihib/leetcode#8 (comment)
There was a problem hiding this comment.
コメント＆補足ありがとうございます。
pyhonって書き方が簡易的なので分かりにくさはありますよね・・・
dict()と{}の違いは全く意識したことがなかったですが、{}の方が速くよく使われるのですね！
dict()を使った際は{}を使うようにメッセージが出るようになっているっぽい。
→これも全く知らなかったです…{}の方がより速いという点で推奨されているようなので、{}を採用するようにします！そのうえで変数名については分かりやすい命名になるよう工夫したいと思います。
| - 有効な答えは必ず存在する | ||
| - どのような解き方ができるか | ||
| - 愚直に配列をループさせて和がtargetとなる要素のindexを返せばいけそう | ||
| - 2つのループを用意する。iは0~n-1、jは1~nまで回す |
There was a problem hiding this comment.
ご指摘ありがとうございます。
考慮できていなかったです。。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.
, の後にスペースを 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.
上記リンク先について知らなかったです…
全く意識できていなかったので、今後は[i, j]と記載したりStyle Guidesを意識した体裁にしていきたいと思います！
7 participants
問題: 1. Two Sum
https://leetcode.com/problems/two-sum/description/
言語: Python