Fixing our DoublyLinkedList#775
Merged
Merged
Conversation
castilma
commented
Apr 2, 2017
|
|
||
| if (this->last == node) { | ||
| this->last = new_node; | ||
| } |
Author
There was a problem hiding this comment.
like insert_after(), I'm not sure, if we want to make insert_*() check for this, or let the caller take responsibility for that case.
| } | ||
| current = current->next; | ||
| } while (current != this->end); | ||
| } while (current != this->last); |
Author
There was a problem hiding this comment.
last and first maybe should be callled back and front.
| void clear() { | ||
| node_t *delete_now = this->first; | ||
| while (this->node_count > 0) { | ||
| node_t *deleted_next = this->first->next; |
castilma
commented
Apr 2, 2017
| void erase(node_t *node) { | ||
| node->previous->next = node->next; | ||
| node->next->previous = node->previous; | ||
| assert(this->node_count != 0); |
| * O(1) | ||
| * @pre \p node belongs to this list. | ||
| */ | ||
| void erase(node_t *node) { |
Author
There was a problem hiding this comment.
Do we want the erase() functions to correct the first/last pointer if necessary?
there can be a protected unlink() function, which does the same without setting first and last.
Member
|
I added that list back when I was writing the heap, mainly as i was toying with fibonacci heaps. If it is unused now, we should just remove it? |
Author
|
I'm a fan of removing code. I'd say yes. |
Member
|
K, go for it :) |
castilma
changed the title
TheJJ
approved these changes
Apr 3, 2017
Member
|
Fixed by removal 😛 |
Contributor
|
Fix by removal is best fix indeed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Our (currently unused) DoublyLinkedList had some real issues.
While writing tests, I discovered many things, but the real interesting things were those, that should have raised a compilation error, but didn't. I guess they didn't, because we defined everything in a header and the functions containing uncompilable code were never called(referenced).
I'm not exactly sure, what we can learn from this.
I would suggest using header files for declaration and a separate file for definition. When trying to create
doubly_linked_list.ofromdoubly_linked_list.cpp, that should have raised an error.Besides that: If we keep everything in one header, doesn't that lead to every compilation unit recreating the machine code for that class?