Skip to content

Conversation

@wojpok
Copy link
Collaborator

@wojpok wojpok commented Nov 3, 2025

This code is part of a pretty-printer PR #256 . This code have been separated from main PR for easier review and merging.

lib/List.fram Outdated
Comment on lines 64 to 73
let rec dropLastAux xs =
match xs with
| [] => impossible ()
| [x] => []
| x :: xs => x :: dropLastAux xs
end in
if isEmpty xs then
None
else
Some (dropLastAux xs)
Copy link
Member

Choose a reason for hiding this comment

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

The invariant that dropLastAux works on non-empty list can be easily ensured by passing the head and the tail of the list separately. In my opinion such an implementation is more clear and doesn't use suspicious impossible function. Same comment applies to other last-related functions, as well as foldRight1* functions.

Suggested change
let rec dropLastAux xs =
match xs with
| [] => impossible ()
| [x] => []
| x :: xs => x :: dropLastAux xs
end in
if isEmpty xs then
None
else
Some (dropLastAux xs)
let rec dropLastAux y xs =
match xs with
| [] => []
| x :: xs => y :: dropLastAux x xs
end in
match xs with
| [] => None
| x :: xs => Some (dropLastAux x xs)
end

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cool trick. I will apply that

@wojpok wojpok requested a review from ppolesiuk November 4, 2025 11:58
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.

2 participants