2
\$\begingroup\$

I have a linked list code in C++:

#include <string>
#include <iostream>

using namespace std;

class Node
{
public:
    string name;
    int age;
    Node *next;
    Node(string name, int age)
    {
        this->name = name;
        this->age = age;
        this->next = nullptr;
    }
};

class List
{
private:
    Node *head;
    int size;

public:
    List()
    {
        this->head = nullptr;
        this->size = 0;
    }

    void insert(string name, int age)
    {
        Node *nodenew = new Node(name, age);
        nodenew->next = nullptr;
        if (this->head == nullptr)
        {
            this->head = nodenew;
        }
        else
        {
            Node *auxi = this->head;
            while (auxi->next != nullptr)
            {
                auxi = auxi->next;
            }
            auxi->next = nodenew;
        }
        this->size = this->size + 1;
    }

    void print()
    {

        if (this->head == nullptr)
        {
            cout << "List is empty"<<endl;
        }

        Node *auxi = this->head;
        cout<<to_string(this->size)+" users in the linked list"<<endl;

        while (auxi != nullptr)
        {

            cout << auxi->name << ", " << auxi->age << endl;
            auxi = auxi->next;
        }
    }

    void deleteList(){//this method
        Node* prev = this->head;
        this->size = 0;
        while (this->head)
        {
            this->head = head->next;
            delete(prev);
            prev = this->head;
        }
        
    }
};

int main()
{
    List linkedList;
    
    linkedList.insert("David", 56);
    linkedList.insert("Susan", 25);
    linkedList.insert("Kim", 41);
    linkedList.insert("Charles", 23);
    linkedList.print();
    linkedList.deleteList();
    //inserting data again after deleting all data in the linked list
    linkedList.insert("Bob", 20);
    linkedList.insert("James", 75);
    linkedList.insert("Carl", 36);
    linkedList.insert("Andy", 78);
    linkedList.deleteList();
    linkedList.print();
    return 0;
}

I get in console the correct output:

4 users in the linked list
David, 56
Susan, 25
Kim, 41
Charles, 23
List is empty
0 users in the linked list

But, I think should be some way to improve my deleteList() method. This method get empty my linked list object. I tried to make it more efficient, but I couldn't. I hope you can help me with that or anything more in my linked list code, thanks.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Defeating namespacing

Don't using namespace std; ever! It pollutes your local namespace. If the standard ever added std::Node, std::List, std::linkedList or any other of your identifiers, your code will break. Leave all std:: namespace identifiers in the std:: namespace.

It is permissible to pull a few std:: namespace identifiers into your local namespace on a case by case basis. Eg)

#include <string>
#include <iostream>

using std::string;
using std::cout;

class Node
{
...

By explicitly pulling in only the ones you are using, you won't run into any surprises when something new is added to the standard.


As mentioned by Toby Speight:

In the using examples, consider showing that using std::cout; could (should) have tighter scope, as it's only used within the print() function. I think that's better practice than bringing it into top-level scope.

#include <string>
#include <iostream>

using std::string;

class Node {
...
};

class List
{
...
    void print()
    {
    using std::cout;

    ...
    }
...
};
...

This is extremely important when it comes to header files. You certainly would not want to force someone who uses your header file have unnecessary identifiers dragged into their scope. For example, if you were to separate your interface and implementations, your interface might look like:

custom_node_list.h:

#include <string>

class Node
{
using std::string;

public:
    string name;
    int age;
    Node *next;
    Node(string name, int age);
};

class List
{
using std::string;

private:
    Node *head;
    int size;

public:
    List();
    void insert(string name, int age);
    void print();
    void deleteList();
};

Now string is only an alias for std::string when inside the Node and List class namespaces. Someone using your classes for their own needs could define their own string without confusion.

Don't endl

When you want a newline, you almost never want << std::endl; since that does two different things.

  1. it adds the \n to the output stream, and
  2. it flushes the stream

The latter operation use usually unnecessary, and can significantly degrade the performance of your program due to unnecessary flushing.

Simply use cout << "List is empty\n";

Unnecessary string conversion

The to_string(this->size) in cout<<to_string(this->size)+" users in ... is unnecessary. You don't have to combine the integer and the string into one larger string for output. Simply emit both into the stream one after the other:

        cout << this->size << " users in the linked list\n";

deleteList efficiency

To your main concern: you're writing to memory unnecessarily:

            this->head = head->next;

If you were deleting just the head node, the above operation would be necessary. However, after deleting the head node, you will be deleting the next head node, and then the next, and so on., each time updating the this memory structure for each step.

The final state will be:

        this->head = nullptr;
        this->size = 0;

there is no need for the intermediate writes.

    void deleteList() {
        Node* node = this->head;
        while (node)
        {
            Node *next = node->next;
            delete node;
            node = next;
        }
        this->head = nullptr;
        this->size = 0;
    }
\$\endgroup\$
1
  • \$\begingroup\$ If the standard ever added? Worse, any implementation already may do so. \$\endgroup\$ Sep 15, 2022 at 18:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.