Return to Answer

deleted 1 character in body

Source Link

edited Aug 14, 2018 at 3:16

hoffmale

Some nitpicks:

Comments are still confusing/distracting
^const_iteratorconst_iterator::operator const_iterator() should be moved to iterator instead (it doesn't make sense to convert a const_iterator to a const_iterator). Also, it should pass before_begin to the const_iterator constructor as well.
Ther return types of iterator::operator->() should be T* instead of T&.
Similarly, the return type of const_iterator::operator->() should be const T*.
The destructor should be noexcept.
While technically allowed, it might surprise users that the move assignment operator extends the lifetime of the elements originally contained in *this.
clear() still doesn't update tail.
emplace doesn't update tail.
emplace has no value to return if both if conditions evaluate to false. Maybe throw an error message?
There's code duplication in push_front and emplace_front. This could be fixed by having push_front call emplace_front internally.
Inconsistent handling of an empty list in pop_front and pop_back: The former throws an exception, whereas the latter silently does nothing.
I'm still not sure if SingleLinkedList<T>::operator<< needs to be part of SingleLinkedList. Since different users will have different opinions on the output format, they'll likely will write their own print functions anyways.

Some nitpicks:

Comments are still confusing/distracting
^const_iterator::operator const_iterator() should be moved to iterator instead (it doesn't make sense to convert a const_iterator to a const_iterator). Also, it should pass before_begin to the const_iterator constructor as well.
Ther return types of iterator::operator->() should be T* instead of T&.
Similarly, the return type of const_iterator::operator->() should be const T*.
The destructor should be noexcept.
While technically allowed, it might surprise users that the move assignment operator extends the lifetime of the elements originally contained in *this.
clear() still doesn't update tail.
emplace doesn't update tail.
emplace has no value to return if both if conditions evaluate to false. Maybe throw an error message?
There's code duplication in push_front and emplace_front. This could be fixed by having push_front call emplace_front internally.
Inconsistent handling of an empty list in pop_front and pop_back: The former throws an exception, whereas the latter silently does nothing.
I'm still not sure if SingleLinkedList<T>::operator<< needs to be part of SingleLinkedList. Since different users will have different opinions on the output format, they'll likely will write their own print functions anyways.

Some nitpicks:

Comments are still confusing/distracting
const_iterator::operator const_iterator() should be moved to iterator instead (it doesn't make sense to convert a const_iterator to a const_iterator). Also, it should pass before_begin to the const_iterator constructor as well.
Ther return types of iterator::operator->() should be T* instead of T&.
Similarly, the return type of const_iterator::operator->() should be const T*.
The destructor should be noexcept.
While technically allowed, it might surprise users that the move assignment operator extends the lifetime of the elements originally contained in *this.
clear() still doesn't update tail.
emplace doesn't update tail.
emplace has no value to return if both if conditions evaluate to false. Maybe throw an error message?
There's code duplication in push_front and emplace_front. This could be fixed by having push_front call emplace_front internally.
Inconsistent handling of an empty list in pop_front and pop_back: The former throws an exception, whereas the latter silently does nothing.
I'm still not sure if SingleLinkedList<T>::operator<< needs to be part of SingleLinkedList. Since different users will have different opinions on the output format, they'll likely will write their own print functions anyways.

Source Link

answered Aug 14, 2018 at 2:27

hoffmale

Some nitpicks:

Comments are still confusing/distracting
^const_iterator::operator const_iterator() should be moved to iterator instead (it doesn't make sense to convert a const_iterator to a const_iterator). Also, it should pass before_begin to the const_iterator constructor as well.
Ther return types of iterator::operator->() should be T* instead of T&.
Similarly, the return type of const_iterator::operator->() should be const T*.
The destructor should be noexcept.
While technically allowed, it might surprise users that the move assignment operator extends the lifetime of the elements originally contained in *this.
clear() still doesn't update tail.
emplace doesn't update tail.
emplace has no value to return if both if conditions evaluate to false. Maybe throw an error message?
There's code duplication in push_front and emplace_front. This could be fixed by having push_front call emplace_front internally.
Inconsistent handling of an empty list in pop_front and pop_back: The former throws an exception, whereas the latter silently does nothing.
I'm still not sure if SingleLinkedList<T>::operator<< needs to be part of SingleLinkedList. Since different users will have different opinions on the output format, they'll likely will write their own print functions anyways.