Skip to content

Conversation

@docto-rin
Copy link
Owner

swap(i, i + 1)

index = len(nums) - 2
previous_max = nums[index + 1]
Copy link

Choose a reason for hiding this comment

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

nums[-1] のほうがシンプルだと思いました。


index = len(nums) - 2
previous_max = nums[index + 1]
while index >= 0 and nums[index] >= previous_max:
Copy link

Choose a reason for hiding this comment

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

previous_max を定義して更新していくより、 nums[index + 1] と比較するほうがシンプルだと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。

その通りですね。
この実装は毎回昇順ソートを維持するように整えているので、

index = len(nums) - 2
while index >= 0 and nums[index] >= nums[-1]:
    send_to_tail(index)
    index -= 1

こうなりますね。(まぁ非効率なんですが...)


swap_target = bisect.bisect_right(nums, nums[pivot], lo=pivot + 1)
assert swap_target < n
swap(pivot, swap_target)
Copy link

Choose a reason for hiding this comment

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

pivot と swap_target を swap してから逆順にする実装をよく見かけます。読み手が理解できるのであれば、どちらでも大丈夫だと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。

そのまま書き換えるなら、下記のような感じでしょうか。

swap_target = (
    bisect.bisect_left(
        nums, -nums[pivot], lo=pivot + 1, key=lambda x: -x
        )
    - 1
)
assert swap_target < n
swap(pivot, swap_target)

reverse_in_section(pivot + 1, n - 1)

ただこれは可読性が低く、ここは計算量的にボトルネックにならないので線形探索とし、

def find_bigger(target, start, stop, step=1):
    for i in range(start, stop, step):
        if nums[i] > target:
            return i
    return stop

swap_target = find_bigger(nums[pivot], n - 1, pivot, -1) 
assert pivot < swap_target < n
swap(pivot, swap_target)

reverse_in_section(pivot + 1, n - 1)

が良い気がしました。

def swap(index1, index2):
nums[index1], nums[index2] = nums[index2], nums[index1]

def is_sorted_until(first, last, step=1):

Choose a reason for hiding this comment

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

関数内関数で、step=-1 以外で使われることがなさそうなので、逆順で見ることを前提とした関数とした方が読みやすいと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。

個人的には反対で、もしstep=-1を関数内にハードコードする場合関数名にstep=-1分の情報を追加する必要があると思います。それよりは、引数でfirst, last, stepを渡す方が可読性が高いと感じます。組み込み関数range()などで比較的馴染みのある引数セットであることが根拠の1つです。

nums.reverse()
return

reverse_in_section(pivot + 1, n - 1)

Choose a reason for hiding this comment

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

reverse してから swap する場合はpivot=-1のケースをまとめて処理するのもありかなと思いました。

        pivot = is_sorted_until(n - 1, 0, -1) - 1
        reverse_in_section(pivot + 1, n - 1)
        if pivot == -1:
            return

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