Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

list-ops: simplify generics #2054

Merged
merged 2 commits into from
Mar 6, 2025
Merged

list-ops: simplify generics #2054

merged 2 commits into from
Mar 6, 2025

Conversation

senekor
Copy link
Contributor

@senekor senekor commented Mar 6, 2025

closes #2053

@Morgane55440
Copy link

Morgane55440 commented Mar 6, 2025

i'd like to also mentions that the changes i proposed slightly change only the first function append

the original append simplified would look like

pub fn append<I : Iterator>(fst: I, snd: I) -> impl Iterator<Item = I::Item>

but i wrote it

pub fn append<I : Iterator, J>(fst: I, snd: J) -> impl Iterator<Item = I::Item>
where
    J : Iterator<Item = I::Item>

because it feels reasonable that it should support different types of input, if you feel it's not worth changing feel free to use the first solution.

Everything else is the exact same, just written differently

@senekor
Copy link
Contributor Author

senekor commented Mar 6, 2025

Yeah I saw that and liked the additional flexibility, but it actually seems to break type inference (not sure why). I'll to figure it out and otherwise revert that particular change.

Edit: just a silly mistake, I wrote <I, J>(_a: I, _b: I) (b shoudl be J not I) so the parameter J was unused.

@senekor senekor force-pushed the senekor/oxkvnnwkzkyp branch from 96852ec to 00ed355 Compare March 6, 2025 14:42
@senekor senekor requested a review from ellnix March 6, 2025 14:45
Copy link
Contributor

@ellnix ellnix left a comment

Choose a reason for hiding this comment

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

because it feels reasonable that it should support different types of input

I really like this as well.

The changes are very good overall, I can't see a problem with them. I think it would be appropriate to add both @Morgane55440 and @senekor as authors or contributors in config.json, since this change is sizable and @senekor came up with the idea of using generics in the first place.

@senekor senekor merged commit 02dce7d into main Mar 6, 2025
10 checks passed
@senekor senekor deleted the senekor/oxkvnnwkzkyp branch March 6, 2025 15:57
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.

the List Ops exercise uses unidiomatic function signatures
3 participants