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

Refactor (update|set|remove|swap|split)At for efficiency #55

Closed
wants to merge 1 commit into from

Conversation

pzp1997
Copy link
Contributor

@pzp1997 pzp1997 commented Jun 5, 2017

I refactored updateAt, setAt, removeAt, swapAt, and splitAt to make them more efficient. All of the functions are now tail-recursive. In the case of success (i.e. the index is in range), the functions perform two passes over the elements from the start of the list up to and including the given index (and in the case of swapAt the larger index). In the case of failure, the functions require only a single pass over the list.

Closes #1 and the second part of #30. I also have a fast implementation for the proposed insertAt function that uses the atHelper, which would close the rest of #30 and #43. I also have another version of the code that changes the return types of updateAt, setAt, and swapAt in accordance with #32 and #36. Just let me know what you need and I can submit additional PRs or add to this one.

@pzp1997
Copy link
Contributor Author

pzp1997 commented Jun 6, 2017

@cbenz Made an interesting point in #36 regarding this PR. Although in theory, these implementations should be faster than the current ones, performance and optimization are a bit tricky in practice. Before we proceed, we should really benchmark the functions so we can truly assess if I was successful. I will try to do this soon. I have also read elm-lang/core#668 about optimizing List.take, which has me wondering if we should take a similar approach. I'm going to close this for now, and re-open it once I have the benchmarks.

@pzp1997 pzp1997 closed this Jun 6, 2017
@pzp1997 pzp1997 mentioned this pull request Jun 26, 2017
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.

splitAt traverses n elements twice
1 participant