##Edit: Based on question comment
Edit: Based on question comment
##Edit: Based on question comment
Edit: Based on question comment
struct Node
{
std::unique_ptr<Node> next;
};
void insert(std::unique_ptr<Node>& after, std::unique_ptr<Node>& newNode)
{
/*
* after: A node in the chain.
* newNode: A new node (with next NULL) that needs to be added to the chain.
*
* This function adds `newNode` to the chain.
* Post condition: after->next = newNode
* newNode->next = after(before call)->next;
*/
std::swap(after->next, newNode->next); // newNode->next now points to the rest of the chain.
std::swap(after->next, newNode); // newNode added to chain.
}
struct Node
{
std::unique_ptr<Node> next;
};
void insert(std::unique_ptr<Node>& after, std::unique_ptr<Node>& newNode)
{
std::swap(after->next, newNode->next); // newNode->next now points to the rest of the chain.
std::swap(after->next, newNode); // newNode added to chain.
}
struct Node
{
std::unique_ptr<Node> next;
};
void insert(std::unique_ptr<Node>& after, std::unique_ptr<Node>& newNode)
{
/*
* after: A node in the chain.
* newNode: A new node (with next NULL) that needs to be added to the chain.
*
* This function adds `newNode` to the chain.
* Post condition: after->next = newNode
* newNode->next = after(before call)->next;
*/
std::swap(after->next, newNode->next); // newNode->next now points to the rest of the chain.
std::swap(after->next, newNode); // newNode added to chain.
}
Everything @ChrisWue said.
Also:
Design.
Using sharded_ptr<>
does have affects on meaning (especially when copying). And I don't think you handle that correctly. Also You are building a container like structure. Usually memory management is done either by container (for groups of values) or by smart pointer (for single values). These are complementary types of memory management and intermixing them adds redundancy. You are already writing code to manage memory why add another layer on top (you can but it usually more efficient to do it manually).
So personally I would have not gone with smart pointers here (because you are building a container).
But the problem:
List a;
// Do stuff with a
List b(a); // Make a copy of `a`
This is not doing what you expect.
With your code both list share the underlying list of nodes. If I modify one of these objects then the other one is modified in parallel. This may be deliberate and feature but it violates the principle of least surprise
and thus should be clearly documented if delibrate.
Note I: Same applies to assignment operator.
Note II: The compiler automatically generates the copy constructor and assignment operator.
I will assume the above was a mistake. Basically I believe you are using the wrong smart pointer. The list of nodes contained by the list is solely owned by the container and there is no shared ownership. A more appropriate smart pointer would have std::unique_ptr<>
. If you would have used this the compiler would have complained when you tried to build as the default copy constructor would have not worked.
The reason for using a smart pointer to control memory management is that it can get quite convoluted in normal code. But because your code hides the link object internally the number of use cases is limited (nobody can abuse your object just you). So this makes memory management simpler and you can quite easily implement it yourself (but std::unique_ptr is a perfectly valid alternative in this case).
Note 3: Implement the Copy constructor. Implement assignment in terms of copy construction by using the Copy and Swap
idiom.
##Edit: Based on question comment
Adding an element to a list that uses std::unique_ptr
struct Node
{
std::unique_ptr<Node> next;
};
void insert(std::unique_ptr<Node>& after, std::unique_ptr<Node>& newNode)
{
std::swap(after->next, newNode->next); // newNode->next now points to the rest of the chain.
std::swap(after->next, newNode); // newNode added to chain.
}
Everything @ChrisWue said.
Also:
Design.
Using sharded_ptr<>
does have affects on meaning (especially when copying). And I don't think you handle that correctly. Also You are building a container like structure. Usually memory management is done either by container (for groups of values) or by smart pointer (for single values). These are complementary types of memory management and intermixing them adds redundancy. You are already writing code to manage memory why add another layer on top (you can but it usually more efficient to do it manually).
So personally I would have not gone with smart pointers here (because you are building a container).
But the problem:
List a;
// Do stuff with a
List b(a); // Make a copy of `a`
This is not doing what you expect.
With your code both list share the underlying list of nodes. If I modify one of these objects then the other one is modified in parallel. This may be deliberate and feature but it violates the principle of least surprise
and thus should be clearly documented if delibrate.
Note I: Same applies to assignment operator.
Note II: The compiler automatically generates the copy constructor and assignment operator.
I will assume the above was a mistake. Basically I believe you are using the wrong smart pointer. The list of nodes contained by the list is solely owned by the container and there is no shared ownership. A more appropriate smart pointer would have std::unique_ptr<>
. If you would have used this the compiler would have complained when you tried to build as the default copy constructor would have not worked.
The reason for using a smart pointer to control memory management is that it can get quite convoluted in normal code. But because your code hides the link object internally the number of use cases is limited (nobody can abuse your object just you). So this makes memory management simpler and you can quite easily implement it yourself (but std::unique_ptr is a perfectly valid alternative in this case).
Note 3: Implement the Copy constructor. Implement assignment in terms of copy construction by using the Copy and Swap
idiom.
Everything @ChrisWue said.
Also:
Design.
Using sharded_ptr<>
does have affects on meaning (especially when copying). And I don't think you handle that correctly. Also You are building a container like structure. Usually memory management is done either by container (for groups of values) or by smart pointer (for single values). These are complementary types of memory management and intermixing them adds redundancy. You are already writing code to manage memory why add another layer on top (you can but it usually more efficient to do it manually).
So personally I would have not gone with smart pointers here (because you are building a container).
But the problem:
List a;
// Do stuff with a
List b(a); // Make a copy of `a`
This is not doing what you expect.
With your code both list share the underlying list of nodes. If I modify one of these objects then the other one is modified in parallel. This may be deliberate and feature but it violates the principle of least surprise
and thus should be clearly documented if delibrate.
Note I: Same applies to assignment operator.
Note II: The compiler automatically generates the copy constructor and assignment operator.
I will assume the above was a mistake. Basically I believe you are using the wrong smart pointer. The list of nodes contained by the list is solely owned by the container and there is no shared ownership. A more appropriate smart pointer would have std::unique_ptr<>
. If you would have used this the compiler would have complained when you tried to build as the default copy constructor would have not worked.
The reason for using a smart pointer to control memory management is that it can get quite convoluted in normal code. But because your code hides the link object internally the number of use cases is limited (nobody can abuse your object just you). So this makes memory management simpler and you can quite easily implement it yourself (but std::unique_ptr is a perfectly valid alternative in this case).
Note 3: Implement the Copy constructor. Implement assignment in terms of copy construction by using the Copy and Swap
idiom.
##Edit: Based on question comment
Adding an element to a list that uses std::unique_ptr
struct Node
{
std::unique_ptr<Node> next;
};
void insert(std::unique_ptr<Node>& after, std::unique_ptr<Node>& newNode)
{
std::swap(after->next, newNode->next); // newNode->next now points to the rest of the chain.
std::swap(after->next, newNode); // newNode added to chain.
}