Skip to content

Make gnome sort generic over comparable items - #15261

Merged
cclauss merged 1 commit into
TheAlgorithms:masterfrom
bpiper02:type-gnome-sort
Sep 10, 2026
Merged

Make gnome sort generic over comparable items#15261
cclauss merged 1 commit into
TheAlgorithms:masterfrom
bpiper02:type-gnome-sort

Conversation

@bpiper02

@bpiper02 bpiper02 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Part of #15234

Updates gnome_sort to use a bounded comparable type instead of an untyped list.

Also adds gnome_sort to the shared test covering rejection of non-comparable mixed items.

Validation:

  • python -m doctest -v sorts/gnome_sort.py
  • python -m pytest tests/test_sorts.py -k "gnome_sort or rejects_non_comparable"
  • python -m ruff check sorts/gnome_sort.py tests/test_sorts.py
  • python -m ruff format --check sorts/gnome_sort.py tests/test_sorts.py
  • python -m compileall sorts/gnome_sort.py

@cclauss

cclauss commented Sep 10, 2026

Copy link
Copy Markdown
Member

@priya-sundaram-dev, your review, please.

@cclauss cclauss added the awaiting reviews This PR is ready to be reviewed label Sep 10, 2026

@priya-sundaram-dev priya-sundaram-dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM ✅

The subtle part here is the guard change from `lst[i - 1] <= lst[i]` to `not lst[i] < lst[i - 1]`, needed because `Comparable` only supplies `lt`. That rewrite is exactly right:

  • Equivalent: for a total order `a <= b` ≡ `not (b < a)`, so the loop advances/swaps identically.
  • Stability preserved: equal elements give `lst[i] < lst[i-1] == False` → advance, no swap — same as before.
  • Protocol-safe: the body now only ever calls `lt`, matching the `Comparable` bound.

I re-ran it locally: 3000 random lists match `sorted`, and a stability check (2000 runs, wrapper comparing on key only) stays stable. Adding `gnome_sort` to the shared `rejects_non_comparable` coverage is the right move too.

Nice, minimal change — consistent with the rest of #15234.

@cclauss
cclauss merged commit 6a8f4ed into TheAlgorithms:master Sep 10, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting reviews This PR is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants