-1

case 1 work fine but case 2 and 3 just stop the loop and the program and i cannot choose any other case after that as! i wonder why it stop the loop from going as choice here would never equal 0 which i believe it's the only reason would stop the loop from going forward! thanks in advance.

i also checked the functions and all of them seems fine to me i'm not sure if the problem could be with them

#include "stdafx.h"
#include <iostream>
using namespace std;
void insert_node(int new_data);
void print_node();
void delete_node();
int new_data;
char choice;
struct node {
    int data;
    node* next;
};

node* head = NULL;

void main()
{
    do
    {

        cout << "Enter 1 to insert a new node \n";
        cout << "Enter 2 to disply the list \n";
        cout << "Enter 3 to delete the last node \n";
        cin >> choice;
        switch (choice)
        {
        case '1':
            cout << "Enter the data \n";
            cin >> new_data;
            insert_node(new_data);
            break;
        case '2':
            if (head != NULL)
                print_node();
            else
                cout << "SORRY, your list is empty \n";
            break;
        case '3':
            delete_node();
            break;

        default:
            cout << "Invalid entry \n";
        }

    } while (choice != '0');
}

void insert_node(int new_data)
{
    node* NewNode = new node;
    NewNode->data = new_data;
    if (head == NULL)
        head = NewNode;
    else
    {
        NewNode->next = head;
        head = NewNode;
    }
}
void print_node()
{
    node* printer=head;
    do
    {
        cout << printer->data<<" - ";
        printer = printer->next;
    } while (printer != NULL);
}
void delete_node()
{
    if (head == NULL)
        cout << "no node to be deleted \n";
    else
    {

        node* curr = head;
        node* prev = NULL;
        while (curr->next != NULL)
        {
            prev = curr;
            curr = curr->next;
        }
        if (prev == NULL)
        {
            delete(curr);
            head = NULL;
            return;
        }
        prev->next = NULL;
        delete(curr);
    }
}
  • 1
    Did you step through your code with a **debugger** to see where that problem occurs, then run it again to watch what happens leading up to that point? – tadman Mar 18 '19 at 21:37
  • You are not checking your input for failure. `if (!(cin >> choice)) throw "Error";` (or throw whatever you prefer) – Eljay Mar 18 '19 at 21:40
  • yes and it says { Exception thrown: red access violation printer was 0xCDCDCDCD) and tbh i'm not sure what does that mean! – Ahmed Hisham Mar 18 '19 at 21:44
  • @AhmedHisham That means you have uninitialised heap memory as explained in my answer. – john Mar 18 '19 at 21:48
  • `0xCDCDCDCD` means uninitialized heap memory. https://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations – drescherjm Mar 19 '19 at 00:09

1 Answers1

1

This is bugged

void insert_node(int new_data)
{
    node* NewNode = new node;
    NewNode->data = new_data;
    if (head == NULL)
        head = NewNode;
    else
    {
        NewNode->next = head;
        head = NewNode;
    }
}

it should be

void insert_node(int new_data)
{
    node* NewNode = new node;
    NewNode->data = new_data;
    NewNode->next = head;
    head = NewNode;
}

Your version fails to set next when adding the first node to the list. And as you can see there's no need to make head == NULL a special case.

Because you aren't creating the list correctly in the first place, any attempt to print or delete items from the list is likely to fail.

And your print_node function would fail on an empty list. It should be a while loop not a do ... while loop.

void print_node()
{
    node* printer=head;
    while (printer != NULL)
    {
        cout << printer->data<<" - ";
        printer = printer->next;
    }
}

Your delete_node function looks very suspicious to me as well.

john
  • 85,011
  • 4
  • 57
  • 81
  • THANK YOU SO MUCH FOR YOUR HELP! i just edited my version with giving next the value null and it worked just fine, i just wonder why do i need to set it null why it doesn't become null by default? and also with the first version before editing why the program printed the list correctly? once again you are a HERO! – Ahmed Hisham Mar 18 '19 at 22:07
  • 1
    @AhmedHisham C++ has a guiding principle of "You don't pay for what you don't need." Automatically setting a dynamically allocated to a safe value (and is `NULL` always a safe value?) variable is a cost that not everyone needs, so if you want a `NULL`, you have to put it there yourself. If you are compiling to a recent C++ Standard (C++11 or better) , you can `struct node { int data; node* next = NULL; };`. If not, you will have to make a constructor that does the deed for you. Sidenote: because `node* head = NULL;` is defined at global scope the compiler will `NULL` this one for you for free. – user4581301 Mar 18 '19 at 22:14