Linked List private pointers C++ [closed]

Question

I am having trouble understanding how to use pointers when they are in "private".

mostly I don't know how to get and set values for pointers

I want to create a head and tail node that have no character value. then create new nodes that lie in between head and tail and add new nodes on to the end of the list (before tail).

the code runs, but it doesn't do anything when I use the print function.

sorry if my formatting is wrong and if the code is too long.

here is the code for my class:

#include <iostream>
using namespace std;

class node
{
public:
  node(void)// constructor for empty nodes
  {
    left_link = NULL;
    right_link = NULL;
  }
  node(char x) // constructor for nodes with given value
  : anything(x)
  { }
  char get_char() // return character
  {
    return anything;
  }
  void setLeftLink(node *left)
  {
    left_link = left;
  }
  void setRightLink(node *right)
  {
    right_link = right;
  }
  node *getlefttLink()
  {
    return left_link;
  }
  node *getRightLink()
  {
    return right_link;
  }

private:
  node *left_link;
  char anything;
  node *right_link;
};

here are my functions:

void append(node *&head, node *&tail);
void print(node *head);


void append(node *&head, node *&tail)
{
  char c;
  cout << "Please enter a single character: ";
  cin >> c;
  node *current = new node(c);
  cout << current->get_char() << endl;
  if(head == NULL && tail == NULL)
  {
    head->setRightLink(current);
    tail->setLeftLink(current);
    current->setLeftLink(head);
    current->setRightLink(tail);
  }
  else
  {
    tail->setRightLink(current);
    current->setLeftLink(tail);
    tail = current;
    tail->setRightLink(NULL);
  }

}
// print function
void print(node *head)
{
  node* temp;
  temp = head;
  while(temp->getRightLink()!=NULL){
    cout<<temp->get_char()<<endl;
    temp = temp->getRightLink();    
  }
}

here is my main:

int main()
{
char choice;
node *head = new node;
node *tail = new node;

cout << "Please choose one menu option at a time:\n" 
    << "1 = Append\n"
    << "2 = Print list\n"
    << "3 = Exit\n\n";


    do
    {
        cout << "Menu option(1-3): ";
        cin >> choice;

        switch (choice)
        {
            case '1': append(head, tail); // add to the end of list.
                break;
            case '2': print(head); // print list
                break;
            case '3': cout << "end program\n\n";
                break;
            default: cout << "try again\n";
                break;
        }


    }while(choice != '3');

return 0;
}

Answer 1

First off, head and tail should be initialized to NULL since there is nothing in the beginning.

node *head = NULL;
node *tail = NULL;

Then change your code to append, particularly the first if statement. change your code to

if(head == NULL && tail == NULL)
{
    head = current;
    tail = current;
}

Since you are starting a new list, both head and tail are the same node that you just inserted.

Finally change the condition of your while loop in your print function. Something as simple as this should work.

while (temp) {
    cout << temp->get_char() << endl;
    temp = temp->getRightLink();
}

You want to print the current node, even it doesn't have a right neighbor.

Answer 2

private members, pointers or otherwise, are data that the object either does not want messed by anyone else with or wants to know if someone messed with them.

Setter and getter methods allow access to the private member, but in a controlled manner. For example if you have a private integer that under no circumstances can ever be greater than ten, you can have code in the setter that checks for a caller trying to force the value out of range and reject the request.

bool setX( int newX)
{
    if (newX > 10)
    {
        return false;
    }
    else
    {
        X = newX;
        return true;
    }
}

Now the program can't have any nasty surprises with X == 11 causing an out-of-range access or whatever.

This is self defense for objects. They maintain control over who sets their data to what and can maintain consistency. Say you have a more complex case where you cannot sample an A/D at over 10000 samples per second with the FIR filter enabled without starving the CPU and locking up the system. Whoops. If the only way to set the Filter state or the sampling rate is through setters in the A/D manager object, the object can test and reject and prevent disaster (and possibly leave a nice fat log message pointing at the bad actor).

Think very hard before implementing a getter that returns a non-constant reference or a pointer. Once the caller has either, they can do whatever they want with the returned data.

The rule of thumb is to default to paranoia: Grant no access to any data without a good reason, and then prefer controlled access through setters and getters.

Onto the specifics of your problem.

Getters and setters for a link node is often a sucker bet. The node most likely cannot determine for itself if a linkage is valid. Only the list manager can. This is a case where the object itself is too ignorant to know what is safe, so you have to open up the internals to another object that knows more. friend is useful here, though it is often better to make the node's links public and never allow the list manager to give a node to a client.

Odds are good the client should know absolutely nothing about how the list works anyway. Read up on coupling.

So the node should be utterly stupid. This means you need to have a ListManager class to (duh) manage the list and protect the nodes from badly behaved actors.

ListManager contains your head, tail, root or whatever along with append and remove, print and other list management methods. Under no circumstances do any of these functions reveal a node to the caller, though they can return a handle or an iterator that can be used to reference a node without giving the caller a tool to damage the list. Iterators are a topic worthy of their own question and probably have quite a few already.

A bit of code to explain the above is in order. Please note I have marked, but not corrected, the logic problems I found. There may be more as this compiles (with C++11 enabled) but I haven't run it.

class ListManager
{
private:
    class node
    {
    public:
        node *left_link = nullptr; // recommendation: immediately set or NULL all 
                                   // pointers unless you have a well documented 
                                   // reason not to and profiling to back it up.
                                   // The time you save can be enormous.
        char anything;
        node *right_link = nullptr;
    };
    node *head = nullptr;
    node *tail = nullptr;

public:

    void append(char c) // head and tail not required ListManager members
    {
        /* removed because the append function should append and only append.
         * If you want to read data from the user, call a read function first 
         * and pass it read character in to append
         * Do one thing and do it well. Every time you add behaviours to a 
         * function, you make it harder to debug. For example, what happens to 
         * the linked list if you fail to read a character? That shouldn't be 
         * append's problem. 
        char c;
        std::cout << "Please enter a single character: ";
        std::cin >> c;
        */
        node *current = new node();

        current->anything = c;
        //std::cout << current->anything << std::endl; removed for same reason as above.

        // think on this: how can head and tail NOT both be NULL at the same time?
        // if you find a way, you have a bug that needs fixing.
        if (head == nullptr && tail == nullptr) 
        {
            // If head is NULL, it has no right_link to assign. This will fail horribly.
            head->right_link = current;
            tail->left_link = current;
            current->left_link = head;
            current->right_link = tail;
            /* Consider instead
            head = current;
            tail = current;
            */ 
        }
        else
        {
            tail->right_link = current;
            current->left_link = tail;
            tail = current;
            tail->right_link = nullptr; // don't need to do this. node constructor 
                                        // ensures current->right_link is NULL
        }
    }
    // print function
    void print() // no parameters required. head is ListManager member
    {
        node* temp;
        temp = head;
        // Again, if head is NULL, temp will be NULL and there will be no right_link
        // consider instead
        // while (temp != nullptr)
        while (temp->right_link != nullptr)
        {
            std::cout << temp->anything << std::endl;
            temp = temp->right_link;
        }
    }
};

Note how node is built right into ListManager and is private. Now only ListManager has access to node and it has complete access.

ListManager also needs a destructor to handle deleteing all of the newed nodes that were appended. It also needs a copy constructor and an assignment operator to make it Rule of Three compliant. "What is The Rule of Three?" you ask? It is very very important. Read the link to save yourself much future debugging. Failure to obey the Rule of Three results in a disproportionate number of C++ questions on Stack Overflow, and there is no point in inflating this statistic further.

Usage:

int main()
{
    ListManager list;
    char choice;
    std::cout << "Please choose one menu option at a time:\n" << "1 = Append\n"
            << "2 = Print list\n" << "3 = Exit\n\n";

    do
    {
        std::cout << "Menu option(1-3): ";
        std::cin >> choice;

        switch (choice)
        {
            case '1':
                list.append('a'); // add to the end of list.
                break;
            case '2':
                list.print (); // print list
                break;
            case '3':
                std::cout << "end program\n\n";
                break;
            default:
                std::cout << "try again\n";
                break;
        }

    } while (choice != '3');

    return 0;
}