7
You have a number of problems with memory management in this linked list. The key thing to remember is that unique_ptr indicates ownership of an object. Use of release, reset, and to a lesser extent get are a code smell: not always wrong, but often an indication that the class is being used incorrectly. Usually you should use swap and move-assignment instead....
5
I think the code does a lot more to reflect its genealogy than it does to represent the actual task. Despite being written in C, it seems to do its best to embody the old line about every sufficiently complex program including a half-broken implementation of half of LISP.
Simply put, even though it's acceptable to a C compiler, it's not really written even ...
4
Move class Node and struct __iterator into class List
It is very weird to see Node::Node<T> inside the code. A Node is an implementation detail of your List, so it should be declared inside class List. The same goes for __iterator. For example:
template<typename T>
class List {
class Node {
T data;
std::unique_ptr<Node> ...
4
Initial comments
static void cleanup(); is a function declaration, but it is not a prototype. It does not give you the protection that a prototype does. For the cleanup() function, it should be static void cleanup(object restored); — for the other functions without a prototype, the argument list should be explicitly written as function(void) to show that ...
3
You are relying on the compiler zeroing local variables. For portability initialize them as needed.
Use the address of either head or next fields (llnode**). Then deletion will become trivial.
Free the deleted node.
head node deletion does not printf. As printf was probably just test code, not so important.
Two loops (instead of ifs) are more readable; first ...
3
In the function del_element_from_last, when removing the first element of the list, you update llist->head. However, when removing the last element of the list, you do not update llist->tail.
If keeping track of the tail of the list is only intended for building the list, and is not intended to be updated by the function del_element_from_last, then you ...
3
I don't know what OS and compiler you use but running gcc -O2 main.c -o main -Wall -Wextra produces several warnings:
main.c: In function ‘del_element_from_last’:
main.c:41:18: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ {aka ‘long unsigned int’} [-Wformat=]
41 | printf("len:%d \n", llist->length);...
1
Replace accumulators by post-processing when possible.
splitAtPredicate :: (a -> Bool) -> [a] -> ([a], [a])
splitAtPredicate p [] = ([], [])
splitAtPredicate p (x:xs)
| p x = ([], xs)
| otherwise = let (left, right) = splitAtPredicate p xs in (x:left, right)
Use existing library functions.
splitAtPredicate p xs = let (left, right) = break p xs ...
1
Welcome to the Haskell community :)
Haskellers like composition. Your logic is composed of 2 parts:
splitWhen :: (a -> Bool) -> [a] -> [[a]]
toTuple :: [[a]] -> ([a], [a])
Let's address splitWhen first.
E.g. splitWhen (== '2') "132342245" should be ["13", "33", "", "45"].
To illustrate how it ...
1
I think you should follow C++ standard insert for containers by passing a begin and end iterator, for example
template<typename T>
void insert(Iterator begin, Iterator begin2, Iterator end2);
void insert(Iterator begin, T value);
1
Here is a pointless cast:
if(k > (size_t)llist->length)
llist->length is already of type size_t. Perhaps you meant (size_t)k > llist->length? But it would be much simpler (and more correct) to change the interface so that k is a size_t to begin with, I think.
A singly-linked list should not need a length member. Consider how you might ...
Only top voted, non community-wiki answers of a minimum length are eligible
