Skip to main content
added 60 characters in body
Source Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

a. Consider replacing Node<T> with:

typedef Node<T> node_t;

...

node_t node;

b. Consider making your implementation STL-enabled. If I were user of this class, the first thing I'd ask you is "how do I iterate over this list?". Doubly-linked lists are great when adding items in the middle, how do I do it? (it's about operations like insertBefore(), insertAfter()). How do I sort it? How do I check if item is on the list? How do I search using predicate?

c. Why do you call the method deleteNode if it's only argument is of type T? It's more to findNodeByValueAndDeleteIt(T).

d. What about assignment operator? You should either declare it private (if you don't support it), or implement it explicitly depending on semantics you want to support (deep copy (most likely what you want), reference, COW, etc):

YourList<int> a; // do something with a

YourList<int> b; // do something with b

a = b; // what is "a" here? what if I add nodes to a? will it affect b?

// imagine, scope ends here, dtor for b is called, everything's fine

// dtor for a is called - here you're going to crash, since

// both a and b were using the same memory and this memory is already

// free, since b's dtor has already been called at this point

e. For methods like void addNode(T data) - why do you pass data by value? Since you store item values by value, in either case you're going to call their copy-ctor. Copy-ctor requires const-reference, so you should consider replacing addNode with void addNode(T const& data).

f. using namespace std; - is it only for cout instead of std::cout in your main()?

  1. Consider replacing Node<T> with:

     typedef Node<T> node_t;
    
     ...
    
     node_t node;
    
  2. Consider making your implementation STL-enabled. If I were user of this class, the first thing I'd ask you is "how do I iterate over this list?". Doubly-linked lists are great when adding items in the middle, how do I do it? (it's about operations like insertBefore(), insertAfter()). How do I sort it? How do I check if item is on the list? How do I search using predicate?

  3. Why do you call the method deleteNode if it's only argument is of type T? It's more to findNodeByValueAndDeleteIt(T).

  4. What about assignment operator? You should either declare it private (if you don't support it), or implement it explicitly depending on semantics you want to support (deep copy (most likely what you want), reference, COW, etc):

     YourList<int> a; // do something with a
    
     YourList<int> b; // do something with b
    
     a = b; // what is "a" here? what if I add nodes to a? will it affect b?
    
     // imagine, scope ends here, dtor for b is called, everything's fine
    
     // dtor for a is called - here you're going to crash, since
    
     // both a and b were using the same memory and this memory is already
    
     // free, since b's dtor has already been called at this point
    
  5. For methods like void addNode(T data) - why do you pass data by value? Since you store item values by value, in either case you're going to call their copy-ctor. Copy-ctor requires const-reference, so you should consider replacing addNode with void addNode(T const& data).

  6. using namespace std; - is it only for cout instead of std::cout in your main()?

a. Consider replacing Node<T> with:

typedef Node<T> node_t;

...

node_t node;

b. Consider making your implementation STL-enabled. If I were user of this class, the first thing I'd ask you is "how do I iterate over this list?". Doubly-linked lists are great when adding items in the middle, how do I do it? (it's about operations like insertBefore(), insertAfter()). How do I sort it? How do I check if item is on the list? How do I search using predicate?

c. Why do you call the method deleteNode if it's only argument is of type T? It's more to findNodeByValueAndDeleteIt(T).

d. What about assignment operator? You should either declare it private (if you don't support it), or implement it explicitly depending on semantics you want to support (deep copy (most likely what you want), reference, COW, etc):

YourList<int> a; // do something with a

YourList<int> b; // do something with b

a = b; // what is "a" here? what if I add nodes to a? will it affect b?

// imagine, scope ends here, dtor for b is called, everything's fine

// dtor for a is called - here you're going to crash, since

// both a and b were using the same memory and this memory is already

// free, since b's dtor has already been called at this point

e. For methods like void addNode(T data) - why do you pass data by value? Since you store item values by value, in either case you're going to call their copy-ctor. Copy-ctor requires const-reference, so you should consider replacing addNode with void addNode(T const& data).

f. using namespace std; - is it only for cout instead of std::cout in your main()?

  1. Consider replacing Node<T> with:

     typedef Node<T> node_t;
    
     ...
    
     node_t node;
    
  2. Consider making your implementation STL-enabled. If I were user of this class, the first thing I'd ask you is "how do I iterate over this list?". Doubly-linked lists are great when adding items in the middle, how do I do it? (it's about operations like insertBefore(), insertAfter()). How do I sort it? How do I check if item is on the list? How do I search using predicate?

  3. Why do you call the method deleteNode if it's only argument is of type T? It's more to findNodeByValueAndDeleteIt(T).

  4. What about assignment operator? You should either declare it private (if you don't support it), or implement it explicitly depending on semantics you want to support (deep copy (most likely what you want), reference, COW, etc):

     YourList<int> a; // do something with a
    
     YourList<int> b; // do something with b
    
     a = b; // what is "a" here? what if I add nodes to a? will it affect b?
    
     // imagine, scope ends here, dtor for b is called, everything's fine
    
     // dtor for a is called - here you're going to crash, since
    
     // both a and b were using the same memory and this memory is already
    
     // free, since b's dtor has already been called at this point
    
  5. For methods like void addNode(T data) - why do you pass data by value? Since you store item values by value, in either case you're going to call their copy-ctor. Copy-ctor requires const-reference, so you should consider replacing addNode with void addNode(T const& data).

  6. using namespace std; - is it only for cout instead of std::cout in your main()?

added 28 characters in body
Source Link

a. Consider replacing Node<T> with:

typedef Node<T> node_t;

...

node_t node;

b. Consider making your implementation STL-enabled. If I were user of this class, the first thing I'd ask you is "how do I iterate over this list?". Doubly-linked lists are great when adding items in the middle, how do I do it? (it's about operations like insertBefore(), insertAfter()). How do I sort it? How do I check if item is on the list? How do I search using predicate?

c. Why do you call the method deleteNode if it's only argument is of type T? It's more to findNodeByValueAndDeleteIt(T).

d. What about assignment operator? You should either declare it private (if you don't support it), or implement it explicitly depending on semantics you want to support (deep copy (most likely what you want), reference, COW, etc):

YourList<int> a; // do something with a

YourList<int> b; // do something with b

a = b; // what is "a" here? what if I add nodes to a? will it affect b?

// imagine, scope ends here, dtor for b is called, everything's fine

// dtor for a is called - here you're going to crash, since

// both a and b were using the same memory and this memory is already

// free, since b's dtor has already been called at this point

e. For methods like void addNode(T data) - why do you pass data by value? Since you store item values by value, in either case you're going to call their copy-ctor. Copy-ctor requires const-reference, so you should consider replacing addNode with void addNode(T const& data).

f. using namespace std; - is it only for cout instead of std::cout in your main()?

Checked nothing from the "algorithmic" standpoint, C++ only.

a. Consider replacing Node<T> with:

typedef Node<T> node_t;

...

node_t node;

b. Consider making your implementation STL-enabled. If I were user of this class, the first thing I'd ask you is "how do I iterate over this list?". Doubly-linked lists are great when adding items in the middle, how do I do it? (it's about operations like insertBefore(), insertAfter()). How do I sort it? How do I check if item is on the list? How do I search using predicate?

c. Why do you call the method deleteNode if it's only argument is of type T? It's more to findNodeByValueAndDeleteIt(T).

d. What about assignment operator? You should either declare it private (if you don't support it), or implement it explicitly depending on semantics you want to support (deep copy, reference, COW, etc):

YourList<int> a; // do something with a

YourList<int> b; // do something with b

a = b; // what is "a" here? what if I add nodes to a? will it affect b?

// imagine, scope ends here, dtor for b is called, everything's fine

// dtor for a is called - here you're going to crash, since

// both a and b were using the same memory and this memory is already

// free, since b's dtor has already been called at this point

e. For methods like void addNode(T data) - why do you pass data by value? Since you store item values by value, in either case you're going to call their copy-ctor. Copy-ctor requires const-reference, so you should consider replacing addNode with void addNode(T const& data).

f. using namespace std; - is it only for cout instead of std::cout in your main()?

Checked nothing from the "algorithmic" standpoint, C++ only.

a. Consider replacing Node<T> with:

typedef Node<T> node_t;

...

node_t node;

b. Consider making your implementation STL-enabled. If I were user of this class, the first thing I'd ask you is "how do I iterate over this list?". Doubly-linked lists are great when adding items in the middle, how do I do it? (it's about operations like insertBefore(), insertAfter()). How do I sort it? How do I check if item is on the list? How do I search using predicate?

c. Why do you call the method deleteNode if it's only argument is of type T? It's more to findNodeByValueAndDeleteIt(T).

d. What about assignment operator? You should either declare it private (if you don't support it), or implement it explicitly depending on semantics you want to support (deep copy (most likely what you want), reference, COW, etc):

YourList<int> a; // do something with a

YourList<int> b; // do something with b

a = b; // what is "a" here? what if I add nodes to a? will it affect b?

// imagine, scope ends here, dtor for b is called, everything's fine

// dtor for a is called - here you're going to crash, since

// both a and b were using the same memory and this memory is already

// free, since b's dtor has already been called at this point

e. For methods like void addNode(T data) - why do you pass data by value? Since you store item values by value, in either case you're going to call their copy-ctor. Copy-ctor requires const-reference, so you should consider replacing addNode with void addNode(T const& data).

f. using namespace std; - is it only for cout instead of std::cout in your main()?

Checked nothing from the "algorithmic" standpoint, C++ only.

added 267 characters in body
Source Link

a. Consider replacing Node<T> with:

typedef Node<T> node_t;

...

node_t node;

b. Consider making your implementation STL-enabled. If I were user of this class, the first thing I'd ask you is "how do I iterate over this list?". Doubly-linked lists are great when adding items in the middle, how do I do it? (it's about operations like insertBefore(), insertAfter()). How do I sort it? How do I check if item is on the list? How do I search using predicate?

c. Why do you call the method deleteNode if it's only argument is of type T? It's more to findNodeByValueAndDeleteIt(T).

d. What about assignment operator? You should either declare it private (if you don't support it), or implement it explicitly depending on semantics you want to support (deep copy, reference, COW, etc):

YourList<int> a; // do something with a

YourList<int> b; // do something with b

a = b; // what is "a" here? what if I add nodes to a? will it affect b?

// imagine, scope ends here, dtor for b is called, everything's fine

// dtor for a is called - here you're going to crash, since

// both a and b were using the same memory and this memory is already

// free, since b's dtor has already been called at this point

e. For methods like void addNode(T data) - why do you pass data by value? Since you store item values by value, in either case you're going to call their copy-ctor. Copy-ctor requires const-reference, so you should consider replacing addNode with void addNode(T const& data).

f. using namespace std; - is it only for cout instead of std::cout in your main()?

Checked nothing from the "algorithmic" standpoint, C++ only.

a. Consider replacing Node<T> with:

typedef Node<T> node_t;

...

node_t node;

b. Consider making your implementation STL-enabled. If I were user of this class, the first thing I'd ask you is "how do I iterate over this list?". Doubly-linked lists are great when adding items in the middle, how do I do it? (it's about operations like insertBefore(), insertAfter()). How do I sort it? How do I check if item is on the list? How do I search using predicate?

c. Why do you call the method deleteNode if it's only argument is of type T? It's more to findNodeByValueAndDeleteIt(T).

d. What about assignment operator? You should either declare it private (if you don't support it), or implement it explicitly depending on semantics you want to support (deep copy, reference, COW, etc):

YourList<int> a; // do something with a

YourList<int> b; // do something with b

a = b; // what is "a" here? what if add nodes to a? will it affect b?

e. For methods like void addNode(T data) - why do you pass data by value? Since you store item values by value, in either case you're going to call their copy-ctor. Copy-ctor requires const-reference, so you should consider replacing addNode with void addNode(T const& data).

f. using namespace std; - is it only for cout instead of std::cout in your main()?

Checked nothing from the "algorithmic" standpoint, C++ only.

a. Consider replacing Node<T> with:

typedef Node<T> node_t;

...

node_t node;

b. Consider making your implementation STL-enabled. If I were user of this class, the first thing I'd ask you is "how do I iterate over this list?". Doubly-linked lists are great when adding items in the middle, how do I do it? (it's about operations like insertBefore(), insertAfter()). How do I sort it? How do I check if item is on the list? How do I search using predicate?

c. Why do you call the method deleteNode if it's only argument is of type T? It's more to findNodeByValueAndDeleteIt(T).

d. What about assignment operator? You should either declare it private (if you don't support it), or implement it explicitly depending on semantics you want to support (deep copy, reference, COW, etc):

YourList<int> a; // do something with a

YourList<int> b; // do something with b

a = b; // what is "a" here? what if I add nodes to a? will it affect b?

// imagine, scope ends here, dtor for b is called, everything's fine

// dtor for a is called - here you're going to crash, since

// both a and b were using the same memory and this memory is already

// free, since b's dtor has already been called at this point

e. For methods like void addNode(T data) - why do you pass data by value? Since you store item values by value, in either case you're going to call their copy-ctor. Copy-ctor requires const-reference, so you should consider replacing addNode with void addNode(T const& data).

f. using namespace std; - is it only for cout instead of std::cout in your main()?

Checked nothing from the "algorithmic" standpoint, C++ only.

edited body
Source Link
Loading
added 175 characters in body
Source Link
Loading
Source Link
Loading