Skip to content

[list-ops]: change // to / and swap foldr arg order #3402

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

Closed
wants to merge 2 commits into from
Closed

[list-ops]: change // to / and swap foldr arg order #3402

wants to merge 2 commits into from

Conversation

IsaacG
Copy link
Member

@IsaacG IsaacG commented Apr 12, 2023

@BethanyG BethanyG changed the title list-ops: change // to / and swap foldr arg order [list-ops]: change // to / and swap foldr arg order Apr 12, 2023
@BethanyG
Copy link
Member

@IsaacG -- you should add your name as a contributor to this exercise. 😄

@BethanyG
Copy link
Member

Can we also put in a comment in the JinJa2 template to maybe remind the next set of folx (more than likely our future selves) why we're swapping arguments? I just think that this will come up again, so any places we can comment for posterity will be helpful. JinJa2 Comments.

@IsaacG
Copy link
Member Author

IsaacG commented Apr 13, 2023

Can we also put in a comment in the JinJa2 template to maybe remind the next set of folx (more than likely our future selves) why we're swapping arguments? I just think that this will come up again, so any places we can comment for posterity will be helpful. JinJa2 Comments.

Sure! I can add that once I get homen in a few hours.

If we are going to break existing solutions, I would vastly prefer to have foldr & foldl both be (acc, el), so that foldl is consistent with Haskell tradition. But I am going to go with your first PR, so we can be done with this debate. 😄

That was actually my first commit. That approach is simpler (less code, no need to swap, no need for a comment). I'm happy to do that, too. It's closer to what is in the problem spec.

@IsaacG
Copy link
Member Author

IsaacG commented Apr 13, 2023

I added a comment to the JinJa2 template.

If you prefer func(acc, el), #3405 does that.

@BethanyG
Copy link
Member

BethanyG commented May 9, 2023

Alright. I am going to go with #3405, and close this one in favor of that. @IsaacG -- can you add yourself as an exercise contributor to that PR? As soon as that is done, I'll approve and merge. Many thanks!

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