1. Two Sum#1
Conversation
naoto-iwase
left a comment
naoto-iwase
left a comment
There was a problem hiding this comment.
お疲れ様です。
初回にも関わらず、さまざまな方法を検討されていて素晴らしいと思いました👍
初回とのことで意図的に多めにコメントさせていただきました。あと取り組み方法についても言及させていただきました。
| if complement in num_to_index: | ||
| return [num_to_index[complement], i] | ||
| num_to_index[num] = i | ||
| raise Exception("Answer does not exists.") |
There was a problem hiding this comment.
Exceptionより、ValueErrorの方がわかりやすく、またキャッチしやすく、親切な気がします。
Exception だとすると、ValueError とかもいいかもしれませんね。
組み込み例外の一覧を一回見ておくといいでしょう。
https://docs.python.org/ja/3/library/exceptions.html#ValueError
https://discord.com/channels/1084280443945353267/1263078966491877377/1296874739377115243
Make use of built-in exception classes when it makes sense. For example, raise a ValueError to indicate a programming mistake like a violated precondition, such as may happen when validating function arguments.
https://google.github.io/styleguide/pyguide.html
https://docs.python.org/ja/3/library/exceptions.html
| nums_length = len(nums) | ||
| left = 0 | ||
| right = nums_length - 1 | ||
| while (left < right) and (left > -1) and (right < nums_length): |
There was a problem hiding this comment.
<, >はandより演算順序が先なので、()はいらないですね。
また、leftは増加しかせず、rightは減少しかしないので、while left < rightだけで良さそうです。
| if complement in num_to_index: | ||
| return [num_to_index[complement], i] | ||
| num_to_index[num] = i | ||
| raise Exception("Answer does not exists.") |
There was a problem hiding this comment.
また、"Answer does not exists"はLeetCodeで問題を解くことに寄っていて、実際にコードベースに組み込んだ時には役に立たない気がします。
ValueError(f"no pair found in nums that sums to target '{target}'")とかでしょうか。
There was a problem hiding this comment.
確かにこの関数を使う人的にはその方がありがたいですね！
| class Solution: | ||
| def twoSum(self, nums: List[int], target: int) -> List[int]: | ||
| num_to_index = [(v, i) for (i, v) in enumerate(nums)] | ||
| num_to_index.sort(key=lambda x: x[0]) |
There was a problem hiding this comment.
.sort()にkey=を与えない場合、要素がタプルなら前の要素から順にkeyとしてくれるので、key=の部分は書かなくても良いと思います。
|
|
||
| class Solution: | ||
| def twoSum(self, nums: List[int], target: int) -> List[int]: | ||
| num_to_index = [(v, i) for (i, v) in enumerate(nums)] |
There was a problem hiding this comment.
v, iより、num, iかnum, indexの方が、変数名と相まってわかりやすいと思います。
また、num_to_indexだと、{num: index}っぽいので、num_and_indexや、nums_with_indicesやnums_indexedなど方が良いと思いました。
| - 時間計算量: O(n^2) | ||
| - 空間計算量: O(1) | ||
|
|
||
|
|
There was a problem hiding this comment.
進め方ですが、Step 2でDiscordで同じ問題をすでに解いているほかの参加者のコードも（見てなければ）見ると良いと思います。また、それぞれを読んだ感想を書いておくとレビュワーの助けになります。
あと、他の人のコードを読むほうが練習として大事なので、Discord サーバー内を漁るといいと思います。
https://discord.com/channels/1084280443945353267/1295357747545505833/1302683501576982608
また、該当問題のコメント集を見ておくのも良いと思います。
| if complement in num_to_index: | ||
| return [num_to_index[complement], i] | ||
| num_to_index[num] = i | ||
|
|
There was a problem hiding this comment.
読みやすいです。
返り値が昇順になるようにしているのも親切だと感じました。
| - 時間計算量: O(n^2) | ||
| - 空間計算量: O(1) | ||
|
|
||
|
|
There was a problem hiding this comment.
おそらくArai60の問題を、LeetCodeの問題番号の小さい順に解かれていると思いますが、単元別に解いた方が学習効率がいいと思います。
特にこだわりがなければ、これを上から順にやるのがおすすめです。
| @@ -0,0 +1,16 @@ | |||
| class Solution: | |||
| def twoSum(self, nums: List[int], target: int) -> List[int]: | |||
| for i, ele_i in enumerate(nums): | |||
There was a problem hiding this comment.
eleはelementの略だと思いますが、英単語から文字を削って変数名にすると読み手にとっては認知負荷が上がる場合があります。（読み手は元の英単語を推測する必要があるため。）原則フルスペルで書くことをお勧めします。
今回、numsが所与なので、numとすればその要素であることがわかるため、num_i, num_jで良いのではないでしょうか。
または、より意味的なquery, complement/partnerなどでも良いと思います。
| - sorted関数を使う方法とsortメソッドを使うがあるらしいが、sortの方が若干効率的だが副作用出す。 | ||
| - ソート前のリストは不要なのでsortを使った（ただ、副作用は予期しないバグを及ぼすので、たとえ今はソート前のリストが不要だとしても副作用は避けた方が良い？） | ||
| - https://docs.python.org/ja/3/howto/sorting.html |
There was a problem hiding this comment.
入力のリストに対し直接.sort()をするのは控えた方がいいと思います。
pythonの関数の引数は参照渡しであり、mutableに対するmutationはスコープ外に波及します。.sort()による破壊的な変更は関数の呼び出しもとに波及し、関数を使った人はびっくりすると思います。
There was a problem hiding this comment.
確かにそうですね！
問題解くことばかり意識が向いていて、この関数を使う人の気持ちを考えられていませんでした。
ありがとうございます。
| def twoSum(self, nums: List[int], target: int) -> List[int]: | ||
| for i, ele_i in enumerate(nums): | ||
| for j, ele_j in enumerate(nums): | ||
| if ele_i + ele_j == target and i != j: |
There was a problem hiding this comment.
ifによる条件式を分けると少し認知負荷が低くなるので良いなと思いました。
以下のような雰囲気です。
| if ele_i + ele_j == target and i != j: | |
| if i == j: | |
| continue | |
| if ele_i + ele_j == target: |
There was a problem hiding this comment.
確かにシンプルですね！
否定も使わなくていいですし。
ありがとうございます！
|
|
||
| ### ソート+2ポインタ法 | ||
| - (index, value)のタプルのリストに変換、value基準で昇順で並べ替え（O(nlogn)して、left=0, right=len(nums)-1から2ポインタ法試す | ||
| - sorted関数を使う方法とsortメソッドを使うがあるらしいが、sortの方が若干効率的だが副作用出す。 |
There was a problem hiding this comment.
若干効率的というときに、時間で見積もったほうがいいように思います。
https://stackoverflow.com/questions/1436962/python-sort-method-on-list-vs-builtin-sorted-function
% python3 -mtimeit -s'import random; x=list(range(1000)); random.shuffle(x)' 'y=list(x); y.sort()'
5000 loops, best of 5: 44.8 usec per loop
% python3 -mtimeit -s'import random; x=list(range(1000)); random.shuffle(x)' 'y=list(x)'
200000 loops, best of 5: 1.21 usec per loop
% python3 -mtimeit -s'import random; x=list(range(1000)); random.shuffle(x)' 'y=list(x); sorted(y)'
5000 loops, best of 5: 46.5 usec per loop
% python3 -mtimeit -s'import random; x=list(range(1000)); random.shuffle(x)' 'sorted(x)'
5000 loops, best of 5: 44.8 usec per loop
% python3 -mtimeit -s'import random; x=list(range(1000)); random.shuffle(x)' 'x.sort()'
100000 loops, best of 5: 2 usec per loop
最後はソート済みなので速いです。
There was a problem hiding this comment.
この時間の解釈ですが、まず、1000 要素のリストのコピーは1マイクロ秒くらい。つまり、1要素がナノ秒くらいです。list のコピーはネイティブコードなので、ほぼ機械語のコピーくらいに思っていいのでしょう。
x.sort() にすると、timeit の仕様上、同じコードを何度も繰り返し呼んで平均を取ります。2回目からは、ソート済みでその時にはソート済みかどうかの確認だけしかしないので速いです。あとは、ほぼ同じですね。45 マイクロ秒について考えましょう。1000要素なので、2^10です。10回程度全体を舐める処理が行われると考えると定数は数ナノ秒ですね。結構速いです。
There was a problem hiding this comment.
なるほどですね。ありがとうございます。
↓の表現を鵜呑みにしてなんとなく良さそうと思って選択してしまっていました。今後はきちんと定量的な判断をしていくように気をつけます。
https://docs.python.org/ja/3/howto/sorting.html
Usually it’s less convenient than sorted() - but if you don’t need the original list, it’s slightly more efficient.
実測値と2 <= nums.length <= 10^4というnumsの制約から、1msec程度しか「若干効率的」はほぼ無視できてあえて副作用を許容するほどのメリットがないことがわかりました。
2 <= nums.length <= 10^4という制約から、高々数百マイクロ秒程度の違いしかない
sorted関数、sortメソッドどちらも 定数倍が数ナノ秒であり計算量はO(n log n)なので、実行時間は 数ナノ秒 * 10,000 * log 10,000 = 数ナノ秒 * 10,000 * 14 = 数百マイクロ秒 程度。
実際計測してみると、見積もりの1/10~10倍の範囲に収まっていた。
%python3 -mtimeit -s'import random; x=list(range(10000)); random.shuffle(x)' 'y=list(x); y.sort()'
200 loops, best of 5: 1.27 msec per loop
% python3 -mtimeit -s'import random; x=list(range(10000)); random.shuffle(x)' 'y=list(x); sorted(y)'
200 loops, best of 5: 1.31 msec per loop
%python3 -mtimeit -s'import random; x=list(range(10000)); random.shuffle(x)' 'y=list(x)'
5000 loops, best of 5: 33.1 usec per loop
10回程度全体を舐める処理が行われると考えると定数は数ナノ秒ですね。結構速いです。
こちらについて感覚を合わせたく確認させてください。
「結構速い」に対する理解は以下であってますでしょうか？
Pythonの1ステップは100ナノ秒~1マイクロ秒くらいかかることもあるが、sorted関数、sortメソッドはどちらもネイティブコードで表現されており、実測値からも1回の処理あたり数ナノ秒で終わっているで、結構速い。
参考：
There was a problem hiding this comment.
「結構速い」で私が暗に比べているのは、C++ でのソートで速度に遜色がないということです。
https://travisdowns.github.io/blog/2019/05/22/sorting.html
が、書いていることはおおむね正しいです。
1msec程度しか「若干効率的」
10マイクロ秒くらいですか? コンソールを立ち上げて上のコマンドを使えば、ローカルに Python 入っていれば試せると思います。
There was a problem hiding this comment.
C++と比べてということだったんですね。なるほどです。
10マイクロ秒くらいですか?
確かにそうでした。ご指摘ありがとうございます。計測したのに見積もりのほうに引っ張られてました。。。
%python3 -mtimeit -s'import random; x=list(range(10000)); random.shuffle(x)' 'y=list(x); y.sort()'
200 loops, best of 5: 1.27 msec per loop
% python3 -mtimeit -s'import random; x=list(range(10000)); random.shuffle(x)' 'y=list(x); sorted(y)'
200 loops, best of 5: 1.31 msec per loop
%python3 -mtimeit -s'import random; x=list(range(10000)); random.shuffle(x)' 'y=list(x)'
5000 loops, best of 5: 33.1 usec per loop
|
@docto-rin
細かくコメントいただき本当にありがとうございます！ コーディング練習会の目的など大事な点についても、自分自身きちんと理解できていなかったことに気づけました。 |
| return [num_to_index[complement], i] | ||
| num_to_index[num] = i | ||
|
|
||
| return [-1, -1] |
There was a problem hiding this comment.
unreachable であることを明示的に示すと、読み手に取って理解しやすくなると思います。 unreachable であることを示す書き方は、過去のレビューコメントを参考にすることをおすすめします。
There was a problem hiding this comment.
5 participants
問題
1. Two Sum
Given an array of integers nums and an integer target, return indices of the two numbers such that they add up to target.
You may assume that each input would have exactly one solution, and you may not use the same element twice.
You can return the answer in any order.
次に解く問題
9. Linked List Cycle