Skip to content

Conversation

@hroc135
Copy link
Owner

@hroc135 hroc135 commented Apr 21, 2025

#### 2a
- step1 の修正
- slices.Delete の内部実装を見たところ、例外処理などのオーバーヘッドがあり、今回は不要だと思ったので、使わなかった
- よく考えたら append(nums[:i], nums[i+1:]...) は O(n-i+1)=O(n) 時間かかっているので、全体で O(n^2) になってしまっていた
Copy link

Choose a reason for hiding this comment

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

Linked List ならばこの操作は得意ですね。
移し替えてみたらどうでしょうか。
https://pkg.go.dev/container/list

Copy link
Owner Author

Choose a reason for hiding this comment

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

お!その発想はなかったです。帰ったら実装してみます。
最近、手元で考えたアルゴリズムをコードに落とし込む力は付いてきたように感じるものの、発想の幅についてはまだまだだと感じています。

Copy link
Owner Author

Choose a reason for hiding this comment

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

書いてみました。

func moveZeroes(nums []int)  {
    numsList := list.New()
    nonzeroCount := 0
    for _, n := range nums {
        if n != 0 {
            numsList.PushBack(n)
            nonzeroCount++
        }
    }
    numsIndex := 0
    for n := numsList.Front(); n != nil; n = n.Next() {
        nums[numsIndex] = n.Value.(int)
        numsIndex++
    }
    clear(nums[nonzeroCount:])
}

Copy link

Choose a reason for hiding this comment

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

あー、いや、それでは別に移す先が []int でも同じですね。一回全部 list に移した後に、頭から n 要素を調べて、0 だったら MoveToBack をするというコードを書けば、step1 と同じアルゴリズムで O(n) でできるという話です。

Copy link
Owner Author

@hroc135 hroc135 Apr 22, 2025

Choose a reason for hiding this comment

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

あ、こういうことですね。

func moveZeroes(nums []int)  {
    numsList := list.New()
    for _, n := range nums {
        numsList.PushBack(n)
    }
    node := numsList.Front()
    for _ = range nums {
        if node.Value.(int) == 0 {
            nextNode := node.Next()
            numsList.MoveToBack(node)
            node = nextNode
        } else {
            node = node.Next()
        }
    }
    node = numsList.Front()
    for i := range nums {
        nums[i] = node.Value.(int)
        node = node.Next()
    }
}

Copy link

Choose a reason for hiding this comment

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

はい、そういうことです。

- 0 でないものを前に持ってくることで、0 がバブルソートぽく後ろに押し出される方法
- > 仮に仕様変更で0ではなくて0以下を端に寄せたいとなった場合にもcontinueする条件をnums[i] <= 0とするだけでワークするので、良いと思います。
- https://github.com/fhiyo/leetcode/pull/54/files#r1720791348
- この視点はなかった。コードの保守性を考慮するというのはこういうことなのか。奥が深い、、
Copy link

@Yoshiki-Iwasa Yoshiki-Iwasa Apr 27, 2025

Choose a reason for hiding this comment

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

個人的には、達成したい目的に依存しない複雑性の排除のほうが保守性には大切かなーと思いました

「もしかしたら役立つかも」という想定は大抵実現しないので、目的を達成するために選択できる手段の中から最もシンプルなものを選ぶほうが保守性は高くなると思います

Choose a reason for hiding this comment

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

もちろん、「シンプル」は実装の容易さや人間の可読性などさまざまな要素があるのでどれが一番シンプルかは単純には決まらないと思いますが。。。

- https://github.com/fhiyo/leetcode/pull/54/files#r1720791348
- この視点はなかった。コードの保守性を考慮するというのはこういうことなのか。奥が深い、、
- Go は `nums[0], nums[0] = nums[0], nums[0]` でも動くが、気持ち悪いので、`i == nonzeroCount` の場合は弾いた
- Goの場合は特別に弾かなくてもよいが、C++ だと未定義動作になるらしい
Copy link

@Yoshiki-Iwasa Yoshiki-Iwasa Apr 27, 2025

Choose a reason for hiding this comment

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

チームの状況次第(C++に慣れている人が多くて驚きがあるなど)だと思いますが、Goで許されているなら気にせずnums[0], nums[0] = nums[0], nums[0]を受け入れてもいい気がします
意味的には別に変なことしてない気がします

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants