Remove value from linked list if it is equal to a parameter Problem: Given a singly linked list of integers l and an integer k, remove all elements from list l that have a value equal to k. 


My solution works, but I suspect it's not smart and optimized because I have many extra variables to solve this question.

Node.h:

#ifndef NODE_H_INCLUDED
#define NODE_H_INCLUDED

 template&lt;typename T&gt;
 struct ListNode {
   ListNode(const T &amp;v) : value(v), next(nullptr) {}
   T value;
   ListNode *next;
};

#endif // NODE_H_INCLUDED


main.cpp:

#include&lt;iostream&gt;
#include "Node.h"

void print(ListNode&lt;int&gt; * l){
    while(l != nullptr){
        std::cout &lt;&lt; l-&gt;value &lt;&lt; "\n";
        l = l-&gt;next;
    }
}

ListNode&lt;int&gt; * removeKFromList(ListNode&lt;int&gt; * l, int k) {
    if(l == nullptr){
        return nullptr;
    }else{
        if(l-&gt;value != k){
            ListNode&lt;int&gt; * L2 = new ListNode&lt;int&gt;(l-&gt;value);
            ListNode&lt;int&gt; * current = L2;
            l = l-&gt;next;
            while(l != nullptr){
                if(l-&gt;value != k){
                    ListNode&lt;int&gt; * L3 = new ListNode&lt;int&gt;(l-&gt;value);
                    while(current-&gt;next != nullptr){
                        current = current-&gt;next;
                    }
                    current-&gt;next = L3;
                }
                l = l-&gt;next;
            }
            return L2;
        }else{
            return removeKFromList(l-&gt;next, k);
        }
    }
}

int main(){
    ListNode&lt;int&gt; * L = new ListNode&lt;int&gt;(5);
    ListNode&lt;int&gt; * L1 = new ListNode&lt;int&gt;(6);
    ListNode&lt;int&gt; * L2 = new ListNode&lt;int&gt;(8);
    L1-&gt;next = L2;
    L-&gt;next = L1;
    print(removeKFromList(L, 7));
    return 0;
}
Thank you for your interest in improving your code! Please, don't be confused about amount of comments. Programming is all about mastering your skills!

What's wrong with the code?

You leak memory!

For each call of new there must be a call for delete. Otherwise objects are not removed. Think of these operators as a C++ wrappers around malloc and free calls.

What could be improved?

Use T type in your methods

You used int k as a function argument, but an actual type you want is T. So it goes:

ListNode&lt;T&gt; * removeKFromList(ListNode&lt;T&gt; * l, T k)


Same for print method.

Use auto

When you change your signature to ListNode&lt;T&gt; notice, that using auto for statements, like

ListNode&lt;int&gt; *L2 = new ListNode&lt;int&gt;(node-&gt;value);


would not require any additional changes in the return types:

auto *L2 = new ListNode&lt;T&gt;(node-&gt;value);


It is common to use lowercase letters for variable names, even though you create all the list objects in uppercase.

Moreover, it looks nicer. But, do not overload your code with auto.
Use it only when it is obvious, what the type of your variable is.
So iterators, lambdas and new operators are the common case.

Variable &amp; method names

Naming method removeKFromList suggests "remove key from the list", so we expect ListNode to contain pairs of key-value. It is confusing to see only T value in the struct, treated as a key in your methods. So it is better to rename method removeAllFromList. All here would stand for all occurrences. Do not use terms "key" and "value" together, unless you mean dictionary. Now: 

ListNode&lt;T&gt; * removeAllFromList(ListNode&lt;T&gt; * l, T value)


It is considered a good practice to name variables in the way it makes sense what they stand for. Anyone should understand their meaning without reading more then couple of lines of code ahead. Ideally, write your code like a book. It wouldn't require much efforts. Try not to use variable names shorter then 3 symbols (except some particular cases, like in for-loop iteration):

ListNode&lt;T&gt; * removeAllFromList(ListNode&lt;T&gt; * list, T value)


Now we see another issue. If you pass ListNode&lt;T&gt; * it is actually a node, that is being passed by, but not the list. Fix:

ListNode&lt;T&gt; * removeAllFromList(ListNode&lt;T&gt; * node, T value)


const &amp;

If integer instantiation is used, it is only a 4 bytes that are copied, so performance changes are unnoticeable. But since we use more complex classes, that would be an issue.
Use const &amp; to fix this:

ListNode&lt;T&gt; *removeAllFromList(const ListNode&lt;T&gt; *node, const T &amp;value)


Also make sure that nodes are not modified by writing const ListNode&lt;T&gt; *node

Style

Usually, it is hard to manually track all your spaces and tabulation. Use code refactoring tools. You sometimes mix styles by writing ) { and ){ with or without a space. We love perfection:)

Nesting levels

When you write:

if(l == nullptr){
    return nullptr;
}else{
   // ...
}


It is better to omit else body. That makes code more readable:

if (l == nullptr) {
    return nullptr;
}
   // ...


Reducing the amount of nested levels is good. And yes, use spaces.

File separation

ListNode methods are better placed in Node.cpp file, because they are relevant to ListNode.

Explicit

You may want to use explicit keyword to avoid unintentional type conversions.

What else could be improved?

Create LinkedList class

It is good to encapsulate your ListNode implementation. Hide your nodes, so no one could modify the values from outside, causing memory leaks or any kind of unintended behavior. It would also allow you to control memory by calling delete on each ListNode element in destructor.

There are more issues I may cover. But, please, start with those, provided above.
The method removeAllFromList could now look like this:

template&lt;typename T&gt;
shared_ptr&lt;ListNode&lt;T&gt;&gt; removeAllFromList(shared_ptr&lt;ListNode&lt;T&gt;&gt; node, const T &amp;value) {
    if (node == nullptr) {
        return nullptr;
    }

    // The beginning node
    shared_ptr&lt;ListNode&lt;T&gt;&gt; root = nullptr;

    // Node used for iteration
    shared_ptr&lt;ListNode&lt;T&gt;&gt; current = nullptr;

    while (node != nullptr) {
        if (node-&gt;value == value) {
            node = node-&gt;next;
            continue;
        }
        if (root == nullptr) {
            root = std::make_shared&lt;ListNode&lt;T&gt;&gt;(node-&gt;value);
            current = root;
        } else {
            current-&gt;next = std::make_shared&lt;ListNode&lt;T&gt;&gt;(node-&gt;value);
            current = current-&gt;next;
        }

        node = node-&gt;next;
    }

    return root;
}


In my answer, I assume that you need to create a copy of the linked list and delete all the nodes with the value provided.