88. Merge Sorted Array by sota009 · Pull Request #1 · sota009/swe-coding-practice

sota009 · 2025-05-08T03:35:30Z

Problem:
- https://leetcode.com/problems/merge-sorted-array/description/
Language:
- Python

oda · 2025-05-08T04:59:51Z

swejp-75/88-merge-sorted-array/memo.md

+    - これを機に用語と概念を整理しておきたい
+- o3に実務っぽいコードを書いてもらった。
+  - エラーハンドリングを入れる
+    - 面接では入力の制約を確認すること。絶対こないとわかっている場合でも書いていいと思うけどnice-to-have?


私は、これはいくつか要素があると思っています。
本当に知りたいことは、一緒に仕事をしてより成果が出せるか、です。
そのために、まずコードがさらっと書けることが大事です。
しかし、そのあとにエラーハンドリングなどは状況に応じて柔軟に考える必要があります。
(たとえば、二分探索でソートされているかを確認しません。エラーを投げて欲しいもの止まって欲しいもの様々です。)
一方で、面接の時間は限られています。なので、本来の仕事であったらエラーハンドリングをしたいとだけ説明して、実際に書くかは相手に委ねるくらいが妥当な線かと思います。

面接が正解を当てるのではなくて「一緒に仕事をしてより成果が出せる」ことを短い時間で伝えるものであるということを意識するといいかと思います。

確かに「一緒に仕事をしてより成果が出せる」は面接中に示したいですね。できるだけ脳内を言語化して、こういうところにも気をつけているのだと伝えていきたいです。

oda · 2025-05-08T05:00:55Z

swejp-75/88-merge-sorted-array/memo.md

+    if m == 0:
+        for i in range(len(nums1)):
+            nums1[i] = nums2[i]
+        return
+    if n == 0:
+        return


これ、実はいらないですかね?

そうなんです。後続のロジックが拾ってくれるから、この２つのifは結果的にいらないんですよね。個人的にはこの関数を読んだ時、先にこのアーリーリターンを書いてくれれば、そのあとのロジックではさっきのケースのことを念頭に置かずに読めると考え、意図的に残すのもありかなと思ったのですがいかがでしょう？それこそ面接中にディスカッションする話題かもしれませんが。

[追記]
このあと右から走らせるポインタを m - 1, n - 1と初期化しているので、無駄に-1になるケースを考えたくないという意図もあり。

あってもいいと思います。m == 0 のほうは私は消すかもしれません。
書き終わった直後に、これなくてもいいですかね、とコメントするとわりと読み慣れている人だなと思います。

oda · 2025-05-08T05:05:34Z

swejp-75/88-merge-sorted-array/memo.md

+
+        # Compare each elements from right
+        while read2_idx >= 0:
+            val1 = nums1[read1_idx]


これ、-1 にアクセスする場合がありませんか。

それと使っていない変数ですね。繰り返し書いたらなぜあるのか違和感を感じる気がします。

これ、-1 にアクセスする場合がありませんか。

read1_idxは

should_pick_from_nums1 = ( read1_idx >= 0 and nums1[read1_idx] > nums2[read2_idx] )

のフラグにより、0からデクリメントされないようになっていると思います。
write_idxは最悪read2_idxと同調しますが、read2_idxはwhile内では0以上なので-1にアクセスされないという認識です。

それと使っていない変数ですね。繰り返し書いたらなぜあるのか違和感を感じる気がします。

これは完全に見直したときに気づくべきでした。無論この変数を作る必要はないです。

[5, 0], [1] とすると、
まず、read1_idx = 0, read2_idx = 0, write_idx = 1 で
should_pick_from_nums1 == True
次のループは
[5, 5], [1]
read1_idx = -1, read2_idx = 0, write_idx = 0
で、
val1 = nums1[read1_idx]
に突入しますね。

この val1, val2 の2行がなければいいですが。

そうですね。頭の中からva1, val2が定義されていないことが勝手に前提になっていました。削除ミスが新たなバグを生む事例をまさに目の当たりにしたので、以後見直しのチェック項目を増やしたいと思います。

oda · 2025-05-08T05:07:04Z

swejp-75/88-merge-sorted-array/memo.md

+  - おなじようにwhileも無理に1つにまとめなくてもいいのではと思ってきた。このあたりは個人差がありそう。
+- はじめてこの練習会に参加したけど、このテンプレで進めるとすごい時間かかる笑 ひとつひとつ熟考したり調べるから力は付きそう
+  - SWEJPは2週目なので、いい塩梅で省略できる箇所は時間を節約していきたい
+


私は、これは、Erase-remove idiom を変形して後ろからやっているという認識をしました。
https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom

なるほど。この概念がイディオムとしてあるんですね。たしかに削除するもの以外をポインタを一回走らせて前に詰める発想は近い気がしますね。私は解いているときに、この問題が書き込みポインタと読み取りポインタを一方向へ動かすという意味で近いなと思ってました。
https://leetcode.com/problems/move-zeroes/description/

rinost081 · 2025-05-08T06:31:30Z

swejp-75/88-merge-sorted-array/memo.md

+        cur1 = nums1[pointer1]
+        cur2 = nums2[pointer2]


ここの"cur"はどういう意味で使われた変数名なのでしょうか？

私の手癖なのですが、配列構造のループ中、現在の取り出している(ポイントしている)変数に対してcurrentの意味でcurと書きました。他の公式解答でcurrやcurと使われている例を見たことがありますが、実務を意識した場合できるだけ内容を明示的して、使うとしてもcur_num1などとしたほうが良さそうと今更ながら思い始めました。

英単語から文字を削って変数名を付けると、認知負荷が上がるように思います。原則フルスペルで書くことをお勧めいたします。

https://google.github.io/styleguide/pyguide.html#316-naming

Avoid abbreviation. In particular, do not use abbreviations that are ambiguous or unfamiliar to readers outside your project, and do not abbreviate by deleting letters within a word.

勉強になります！はっきり規約で書いてくれるのはありがたいですね。今後はこの規約ベースで練習を進めたいと思います。

huyfififi

久々にSotaさんのコードが読めて嬉しいです。様々なことが考察されていてとても勉強になりました。ありがとうございます！

huyfififi · 2025-05-08T06:16:19Z

swejp-75/88-merge-sorted-array/memo.md

+  - 既にストアされている要素のアドレスを動かしてはいけないということかと気づき、素直に要素ごとにアクセスする
+- 既にどっちも昇順なので、ポインタをどちらかから走らせ、ひとつひとつ比べ、適切な箇所にストアしていく
+  - 左から見ていくと、num1の既にある要素を上書きしてしまう
+  - 0のスペースが右に空けられているので、右からポインタをはしらせよう


これは単純な興味なのですが、こういった場合に右からポインタを走らせるのって自前ですぐに思いつけるものなのですか？
私はこの問題を解いたときなんとか左から走らせて解けないか考えてかなりの時間を浪費してしまいました 😓

正直特有のテクニックというか初見だと難しいと思います😭 私の場合、たいがい左から試して無理そうならとりあえず逆を発想するようにしてます。SWEJP-75だと

https://leetcode.com/problems/valid-palindrome/ (中心から)

https://leetcode.com/problems/product-of-array-except-self/description/ (左からやって、つぎ右から)

https://leetcode.com/problems/container-with-most-water/ (両端から)

https://leetcode.com/problems/move-zeroes/ (左からwriteとreadを走らせる。この問題に似てるかも)
あたりでポインタの走らせ方の考察をした覚えがあります。

確かに発想が少しいる気はしますが、ただ、たとえば、[1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 0, 0] の先頭に -1 を2つ入れろといわれたら後ろからやりませんか。後ろが空いているのだから後ろから処理するというのは自然に思います。

Sotaさん、丁寧に問題へのリンクも提示していただいてありがとうございます！
Odaさん、感覚の共有ありがとうございます！まだ掴みきれていない部分だったので精進しようと思います

huyfififi · 2025-05-08T06:19:14Z

swejp-75/88-merge-sorted-array/memo.md

+    if n == 0:
+        return
+
+    store_pointer = len(nums1) - 1


個人的な好みですが、pointer1とpointer2の初期化に与えられた n・mが使われているので、store_pointerもm + n - 1とした方が統一感があり、pointer1, pointer2との関係性もわかりやすく、m, nを使用しない理由を考えさせないので良いかなと思いました。大きな問題ではないですが...

問題の意味とコードの統一性を考えるとその通りですね。ここは例の図解を見て、nums1の右端にポインターを置きたい-> len(nums1)- 1と飛びついた結果でした。

huyfififi · 2025-05-08T06:27:51Z

swejp-75/88-merge-sorted-array/memo.md

+    pointer1 = m - 1
+    pointer2 = n - 1
+    while store_pointer >= 0 and pointer1 >= 0 and pointer2 >= 0:
+        cur1 = nums1[pointer1]


currentだけだと、現在の、「何」、という情報が欠けていてぱっと見数字なのかポインタなのかわかりづらいので、num1とする方が好みです。

自戒してました。
#1 (comment)

このあたりいつも迷ってしまいますね。よく配列の変数名を複数形にして、現在取り出しているものを単数形にして表しますが、sだけの差異だと例えばnumsなのかnumか見間違えるミスが過去に何度かあり、いまだにベストプラクティスがわかってないです。Kazukiｋんここでcur_num1を使うのはどう思います？あんまり自然じゃないですかね？

@huyfififi 既に読まれているかもしれませんが、有益なコーディング規約の共有をいただきました。
#1 (comment)

コーディング規約については、コメント集にまとめてあります。
https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.9xjrags8izok

私が好ましいと感じている態度は、コーディング規約がいくつかあることを知っていて、その上で自分の好みを決めているが、チーム内の合意あるいはテックリードの意見に合わせます、というものです。

odaさん共有ありがとうございます。

唯一解がないものに対して慣習を知っている

自分の意見(好み)がある

その上で環境に応じて柔軟に/素直に選択する

という態度ですね。日頃から意識したいです。面接でもそれが伝わるようにコミュニケーションできればと思います。

Kazukiさんここでcur_num1を使うのはどう思います？あんまり自然じゃないですかね？

num, numsだけで区別しようとすると、見間違えるリスクは確かにありますよね。私は、「現在注目している」というのはある程度自明なのでcurはない方が好みなのですが、cur_num1とする気持ちも理解できます！

参考までに、currentという変数名について、私がたびたび受ける指摘へのリンクを貼っておきますね
Mike0121/LeetCode#7 (comment)
olsen-blue/Arai60#2 (comment)
onyx0831/leetcode#1 (comment)

助かります！LeetCodeの解答やdiscussionを見てcurrentが勝手に業界スタンダードだと思ってた節がありました。たしかに実務系のコードではあまりみたことないかもしれませんね。他の人のPR見るのも相当勉強になるというか、色々な意見を見ることができて発見があり、どんどん掘っていこうと思います。

huyfififi · 2025-05-08T06:29:56Z

swejp-75/88-merge-sorted-array/memo.md

+- whileに別にstore_pointerの条件いらない
+  - 2つのポインタが0以上なら自ずとstore_pointerもvalid
+- for ... in range()について
+    - スペースは食わないみたい


スペース食わなそうですね

Python Documentation - Built-in Types

The advantage of the range type over a regular list or tuple is that a range object will always take the same (small) amount of memory, no matter the size of the range it represents (as it only stores the start, stop and step values, calculating individual items and subranges as needed).

cpython/cpython - rangeobject.c

typedef struct { PyObject_HEAD PyObject *start; PyObject *stop; PyObject *step; PyObject *length; } rangeobject;

Python2 までは、range と xrange があって、range はリストを生成していて、xrange はしていませんでしたが、Python3 から、xrange が廃止されて、range が旧 xrange のようになりました。

古いLeetCodeの解答でxrangeを使っているものを見たことがありましたが、そもそも動きかたが全然違うんですね。さすがPythonは2系と3系で大きく変わったと言われるだけありますね

huyfififi · 2025-05-08T06:34:23Z

swejp-75/88-merge-sorted-array/memo.md

+        """
+        Do not return anything, modify nums1 in-place instead.
+        """
+        if m < 0 or n < 0 or m + n != len(nums1) or n != len(nums2):


私ならこれを呼び出し側の責任にしてassertを使いますね。この関数への入力を外部からAPIから受けるとかだったらValueErrorの方が適切だと思います。

どこで使われる想定なのかは重要な視点ですね。面接では相手がそこまで想定していない可能性があるので、こっちから「◯◯で使うと仮定していいですか？」とコントロールしてもいいかもですね。

ちなみに私は昔フィードバックで、assertは実務でテスト以外にエラーの送出に使うなと変更させられたことがありました😅 そう書いてあるドキュメントもどこかで見たことがある気がしますが、ライブラリのエラーで使われているケースもみたことがあるので、一概に禁止しなくてもいいかなと個人的に思ってます。今回は呼び出し側の責任という意見に同意なのでassertでもいいと感じました。

少し古いStackOverflowの回答ですが、internalな挙動についてコメントやドキュメントを残すよりもassertを残した方が読み手に対する強いメッセージになる、のような指摘を読んで、なるほどなと思った記憶があります。

Google Testing Blog: To Assert or Not To Assertでは、nullをassertすることに対して意見が述べられていますが、私が注目したのは

I am not against asserts, I often use them in my code as well, but most of my asserts check the internal state of an object

という部分で、テスト以外でもassertを使っていいんだな、と受け取りました。

あともう一つ assertで落とすことについて参考になったMasaki Haraさんの勉強会資料があったような気がするのですが、こちらは見つかりませんでした 😅

個人的には上記３つを見て、return earlyする気持ちで結構軽率にassertを入れていますが、業界スタンダードかはわかりかねますね...

Sotaさんが受けたフィードバック、参考になります！自分のプロジェクト経験だけだとどうしても思想が偏りがちなので、キャリブレーションできて嬉しいです。

参考リンクありがたいです！たしかに強いメッセージ性が残せるのは同意ですね。過去のコメント集でassertに関しodaさんのフィードバックを見つけました。

受け取らなくて良く、後に編集する開発者に制約を明示するという目的ならば、
assert を入れる、というのは一つあります。assert はデバッグ時のみ走る条件で、実態は、条件と例外です。
fuga-98/arai60#22 (comment)

同時に "assert はデバッグ時のみ走る条件" という点について気になり調べてみたら、pythonのoptimization mode (-O option) で assertは無効化されるようですね。
https://realpython.com/python-assert-statement/#running-python-with-the-o-or-oo-options

それを踏まえてか、Googleのガイドラインではこんなことが書いてありました。

Do not use assert statements in place of conditionals or validating preconditions. They must not be critical to the application logic. A litmus test would be that the assert could be removed without breaking the code. assert conditionals are not guaranteed to be evaluated.
https://google.github.io/styleguide/pyguide.html#244-decision

やはりこれも

1. 唯一解がないものに対して慣習を知っている 2. 自分の意見(好み)がある 3. その上で環境に応じて柔軟に/素直に選択する

の領域かもしれませんね。

huyfififi · 2025-05-08T06:38:07Z

swejp-75/88-merge-sorted-array/step2.py

+            pick_from_nums1 = (
+                read1_idx >= 0 and nums1[read1_idx] > nums2[read2_idx]
+            )
+            if pick_from_nums1:


まだ許容範囲内ですが、最後に合流するまで少し間が空いて脳内メモリに情報を保持しておく負荷がちょっとかかる感覚があるので、

if pick_from_nums1: nums1[write_idx] = nums1[read1_idx] read1_idx -= 1 write_idx = -= 1 continue

としてしまって、読み手の脳内メモリを開放する手法もあるかなと思います。write_idx -= 1を共通化する差し迫った理由もなさそうですし。

ここも派閥が別れますよね！私は一時期、アーリーリターンやコンティニューを乱発していた時期がありました(このコメントの発想もおそらくこの名残 #1 (comment) )。そのときテックリードに読みづらいと言われ、そこから少し改宗(?)して、脳内の負荷で決めるようになりました。
ここではifとelseでやることは同じで、ともにポインタをひとつ下げたいという流れがあるので、個人的にそこまで負荷が強くないかなと思い共通化しました。ただあえて分けておくのも読みやすいと思ってます！

このあたりは面接対策の場合、その会社発のリポジトリを読んでおくなどして、好まれる傾向を見ておくのもいいかもですね。それでも個人差はあると思いますが。

私はこれは if else でもいいかなと思います。write_idx -= 1 をくくることについては私は迷いましたね。趣味の範囲かなと思います。

huyfififi · 2025-05-08T06:40:40Z

swejp-75/88-merge-sorted-array/memo.md

+        read1_idx = m - 1
+        read2_idx = n - 1
+
+        while read2_idx >= 0:


私も解いてみたらSotaさんのStep 1と同じようなコードになりましたが、言われてみれば確かに、while二つもいらないですね 👀

oda · 2025-05-08T14:30:04Z

個人的には、指摘に納得した箇所のソースコードを直さないほうがよいと思っています。
これは練習で後で見たときの学習効率を上げたいです。

sota009 · 2025-05-08T17:30:04Z

個人的には、指摘に納得した箇所のソースコードを直さないほうがよいと思っています。これは練習で後で見たときの学習効率を上げたいです。

たしかに！コミット履歴を遡るのは面倒ですからね。今後もやり方を変えていくかもしれませんが、次回はstep3までのコード(レビューを受けるコード)はそのまま残しつつ、必要であればフィードバック反映後の最終コードを別のstep4かどこかに入れようと思います。
指摘を受けて納得したり学びがあったコメントや自分の反省はども収集して、あとで振り返れるように個人的にまとめておこうと思います。

sota009 added 3 commits May 7, 2025 20:32

Completed step 1, 2 and 3

064c467

Fix indentation in .md file

3ddd4f2

Fix typo

7516f82

oda reviewed May 8, 2025

View reviewed changes

Remove unused variables

92b6f52

rinost081 reviewed May 8, 2025

View reviewed changes

huyfififi approved these changes May 8, 2025

View reviewed changes

sota009 added 2 commits May 8, 2025 00:42

Use assert + make variable initialization more consistent

337b5b2

Fix typo

0972ed9

sota009 merged commit 8a75c35 into main May 9, 2025

sota009 deleted the 88-merge-sorted-array branch May 9, 2025 05:01

Conversation

sota009 commented May 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sota009 May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huyfififi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sota009 May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huyfififi May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huyfififi May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sota009 May 8, 2025 •

edited

Loading

sota009 May 8, 2025 •

edited

Loading

huyfififi May 8, 2025 •

edited

Loading

huyfififi May 8, 2025 •

edited

Loading

sota009 May 8, 2025 •

edited

Loading

sota009 commented May 8, 2025 •

edited

Loading