Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upHacktoberfest: Update Linked List - `print_reverse` method #2792
Conversation
Use a generator expression instead of slicing `elements_list` to improve the space and time complexity of `make_linked_list` to O(1) space and O(n) time by avoiding the creation a shallow copy of `elements_list`.
Add argument typing to all methods in `print_reverse` Add doctest to helper function `make_linked_list` and basic edge case tests to `print_reverse`
Fix doctest syntax and remove edge case tests that are covered by typed arguments. Add `print_reverse` test that expects the correct values are printed out by adding a `test_print_reverse_output` helper function.
c695884
to
4b1e4f8
|
@shermanhui I formatted the code. Hope you do not mind |
| for data in elements_list[1:]: | ||
| current.next = Node(data) | ||
| current = head = Node(elements_list[0]) | ||
| for i in range(1, len(elements_list)): |
shermanhui
Oct 5, 2020
Author
Contributor
@shellhub Thanks for the review and merge, I was hoping to improve the algorithm by using the generator as a part of my Hacktoberfest contribution for #2510. What was the reason for removing it for the regular iteration over a list?
i.e. (what I had before)
for data in (elements_list[i] for i in range(1, list_length)):
shellhub
Oct 5, 2020
•
Member
def fun1() -> None:
my_list = []
elements_list = [14, 52, 14, 12, 43]
for i in range(0, len(elements_list)):
my_list.append(i)
def fun2() -> None:
my_list = []
elements_list = [14, 52, 14, 12, 43]
for data in (elements_list[i] for i in range(0, len(elements_list))):
my_list.append(data)
if __name__ == '__main__':
import timeit
print(timeit.timeit("fun1()", setup="from __main__ import fun1", number=100000))
print(timeit.timeit("fun2()", setup="from __main__ import fun2", number=100000))output:
0.087008857
0.119698832
shermanhui
Oct 5, 2020
•
Author
Contributor
Thanks, good catch, @shellhub. That was informative! I only ran timeit against the list slicing. I should have checked the regular iteration as well. I guess it's also better to keep it simple!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Describe your change:
Update
make_linked_listhelper to use a generator expression instead ofslicing the original
elements_listto avoid creating a temporary listto provide an example that works in O(1) space and O(n) time complexity.
This change is on L46.
I've also added doctests to this file to add test coverage for
expected behaviour and edge cases. When adding the tests, I also
added an additional type check in
print_reverseL58 as I discoveredthe function would break when providing an argument that is not a
Nodeinstance.I've checked off "Fix a bug or typo in an existing algorithm" as it
seems to be the option that closest reflects what I've done in this PR.
Checklist:
Fixes: #{$ISSUE_NO}.