-3

I was solving this problem on Linked List on hacker rank : https://www.hackerrank.com/challenges/insert-a-node-at-the-head-of-a-linked-list/

And got correct answer for the given code . Most of the code was prewritten ; I only had to complete the function insertNodeAtHead function (enclosed between the three stars) :

#include <bits/stdc++.h>
using namespace std;

class SinglyLinkedListNode {
    public:
        int data;
        SinglyLinkedListNode *next;

        SinglyLinkedListNode(int node_data) {
            this->data = node_data;
            this->next = nullptr;
        }
};

class SinglyLinkedList {
    public:
        SinglyLinkedListNode *head;
        SinglyLinkedListNode *tail;

        SinglyLinkedList() {
            this->head = nullptr;
            this->tail = nullptr;
        }

};

void print_singly_linked_list(SinglyLinkedListNode* node, string sep, ofstream& fout) {
    while (node) {
        fout << node->data;

        node = node->next;

        if (node) {
            fout << sep;
        }
    }
}

void free_singly_linked_list(SinglyLinkedListNode* node) {
    while (node) {
        SinglyLinkedListNode* temp = node;
        node = node->next;

        free(temp);
    }
}

// Complete the insertNodeAtHead function below.

/*
 * For your reference:
 *
 * SinglyLinkedListNode {
 *     int data;
 *     SinglyLinkedListNode* next;
 * };
 *
 */
***SinglyLinkedListNode* insertNodeAtHead(SinglyLinkedListNode* llist, int data) {

  SinglyLinkedListNode *nnode;
  nnode = new SinglyLinkedListNode(data);
  if(llist !=NULL)
    nnode->next=llist;
  return llist=nnode;
}***

int main()
{
    ofstream fout(getenv("OUTPUT_PATH"));

    SinglyLinkedList* llist = new SinglyLinkedList();

    int llist_count;
    cin >> llist_count;
    cin.ignore(numeric_limits<streamsize>::max(), '\n');

    for (int i = 0; i < llist_count; i++) {
        int llist_item;
        cin >> llist_item;
        cin.ignore(numeric_limits<streamsize>::max(), '\n');

        SinglyLinkedListNode* llist_head = insertNodeAtHead(llist->head, llist_item);
        llist->head = llist_head;
    }

    print_singly_linked_list(llist->head, "\n", fout);
    fout << "\n";

    free_singly_linked_list(llist->head);

    fout.close();

    return 0;
}

I had to complete the insertNodeAtHead() function only . Rest all was already there. But when I tried to do it without using pointer nnode object( my identifier) of SinglyLinkedListNode class type(their predefined class) I get compilation error:

#include <bits/stdc++.h>

using namespace std;

class SinglyLinkedListNode {
  public:
    int data;
  SinglyLinkedListNode * next;

  SinglyLinkedListNode(int node_data) {
    this - > data = node_data;
    this - > next = nullptr;
  }
};

class SinglyLinkedList {
  public:
    SinglyLinkedListNode * head;
    SinglyLinkedListNode * tail;

  SinglyLinkedList() {
    this - > head = nullptr;
    this - > tail = nullptr;
  }

};

void print_singly_linked_list(SinglyLinkedListNode * node, string sep, ofstream & fout) {
  while (node) {
    fout << node - > data;

    node = node - > next;

    if (node) {
      fout << sep;
    }
  }
}

void free_singly_linked_list(SinglyLinkedListNode * node) {
  while (node) {
    SinglyLinkedListNode * temp = node;
    node = node - > next;

    free(temp);
  }
}

// Complete the insertNodeAtHead function below.

/*
 * For your reference:
 *
 * SinglyLinkedListNode {
 *     int data;
 *     SinglyLinkedListNode* next;
 * };
 *
 */
***SinglyLinkedListNode * insertNodeAtHead(SinglyLinkedListNode * llist, int data) {

  SinglyLinkedListNode nnode;
  nnode = new SinglyLinkedListNode(data);
  if (llist != NULL)
    nnode.next = llist;
  return llist = & nnode;
}***

int main() {
  ofstream fout(getenv("OUTPUT_PATH"));

  SinglyLinkedList * llist = new SinglyLinkedList();

  int llist_count;
  cin >> llist_count;
  cin.ignore(numeric_limits < streamsize > ::max(), '\n');

  for (int i = 0; i < llist_count; i++) {
    int llist_item;
    cin >> llist_item;
    cin.ignore(numeric_limits < streamsize > ::max(), '\n');

    SinglyLinkedListNode * llist_head = insertNodeAtHead(llist - > head, llist_item);
    llist - > head = llist_head;
  }

  print_singly_linked_list(llist - > head, "\n", fout);
  fout << "\n";

  free_singly_linked_list(llist - > head);

  fout.close();

  return 0;
}

ERROR AFTER COMPILATION:

solution.cc: In function ‘SinglyLinkedListNode* insertNodeAtHead(SinglyLinkedListNode*, int)’: solution.cc:63:24: error: no matching function for call to ‘SinglyLinkedListNode::SinglyLinkedListNode()’ SinglyLinkedListNode nnode; ^~~~~ solution.cc:10:9: note: candidate: ‘SinglyLinkedListNode::SinglyLinkedListNode(int)’ SinglyLinkedListNode(int node_data) { ^~~~~~~~~~~~~~~~~~~~ solution.cc:10:9: note: candidate expects 1 argument, 0 provided solution.cc:5:7: note: candidate: ‘constexpr SinglyLinkedListNode::SinglyLinkedListNode(const SinglyLinkedListNode&)’ class SinglyLinkedListNode { ^~~~~~~~~~~~~~~~~~~~ solution.cc:5:7: note: candidate expects 1 argument, 0 provided solution.cc:5:7: note: candidate: ‘constexpr SinglyLinkedListNode::SinglyLinkedListNode(SinglyLinkedListNode&&)’ solution.cc:5:7: note: candidate expects 1 argument, 0 provided solution.cc:64:40: error: ambiguous overload for ‘operator=’ (operand types are ‘SinglyLinkedListNode’ and ‘SinglyLinkedListNode*’) nnode = new SinglyLinkedListNode(data); ^ solution.cc:5:7: note: candidate: ‘SinglyLinkedListNode& SinglyLinkedListNode::operator=(const SinglyLinkedListNode&)’ class SinglyLinkedListNode { ^~~~~~~~~~~~~~~~~~~~ solution.cc:5:7: note: conversion of argument 1 would be ill-formed: solution.cc:64:11: error: invalid user-defined conversion from ‘SinglyLinkedListNode*’ to ‘const SinglyLinkedListNode&’ [-fpermissive] nnode = new SinglyLinkedListNode(data); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ solution.cc:10:9: note: candidate is: ‘SinglyLinkedListNode::SinglyLinkedListNode(int)’ SinglyLinkedListNode(int node_data) { ^~~~~~~~~~~~~~~~~~~~ solution.cc:10:9: note: conversion of argument 1 would be ill-formed: solution.cc:64:11: error: invalid conversion from ‘SinglyLinkedListNode*’ to ‘int’ [-fpermissive]
nnode = new SinglyLinkedListNode(data); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ solution.cc:64:11: error: invalid conversion from ‘SinglyLinkedListNode*’ to ‘int’ [-fpermissive] solution.cc:10:34: note: initializing argument 1 of ‘SinglyLinkedListNode::SinglyLinkedListNode(int)’ SinglyLinkedListNode(int node_data) { ~~~~^~~~~~~~~ solution.cc:5:7: note: candidate: ‘SinglyLinkedListNode& SinglyLinkedListNode::operator=(SinglyLinkedListNode&&)’ class SinglyLinkedListNode { ^~~~~~~~~~~~~~~~~~~~ solution.cc:5:7: note: conversion of argument 1 would be ill-formed: solution.cc:64:11: error: invalid user-defined conversion from ‘SinglyLinkedListNode*’ to ‘SinglyLinkedListNode&&’ [-fpermissive] nnode = new SinglyLinkedListNode(data); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ solution.cc:10:9: note: candidate is: ‘SinglyLinkedListNode::SinglyLinkedListNode(int)’ SinglyLinkedListNode(int node_data) { ^~~~~~~~~~~~~~~~~~~~ solution.cc:10:9: note: conversion of argument 1 would be ill-formed: solution.cc:64:11: error: invalid conversion from ‘SinglyLinkedListNode*’ to ‘int’ [-fpermissive]
nnode = new SinglyLinkedListNode(data); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ solution.cc:64:11: error: invalid conversion from ‘SinglyLinkedListNode*’ to ‘int’ [-fpermissive] solution.cc:10:34: note: initializing argument 1 of ‘SinglyLinkedListNode::SinglyLinkedListNode(int)’ SinglyLinkedListNode(int node_data) { ~~~~^~~~~~~~~ solution.cc:64:40: error: conversion to non-const reference type ‘class SinglyLinkedListNode&&’ from rvalue of type ‘SinglyLinkedListNode’ [-fpermissive] nnode = new SinglyLinkedListNode(data);

Exit Status 255

I firmly believed that it should have worked but it didn't happen so. Since the logic was same , the only difference was I didn't use pointer to create the object. Since I am new to C++ I am unable to figure out why is this happening. Please give an insight.

user4581301
  • 33,082
  • 7
  • 33
  • 54
  • Look at the line the error is on. `SinglyLinkedListNode nnode;` This doesn't do what you think. – Raymond Chen Jun 05 '19 at 02:34
  • 3
    Warning`#include ` [is a bad idea](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h). `using namespace std;` [is a bad idea](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). Combining the two is a really bad idea. – user4581301 Jun 05 '19 at 02:36
  • Warning: `- >` is not a valid operator. Use `->`. – user4581301 Jun 05 '19 at 02:44
  • 5
    Don't learn programming from sites like these if you ever want to do real non-competitive programming. They may look high-quality with code that appears to work fine, but a lot of the times the stuff they teach is utter crap. The code here not only contains very bad practices but contains [undefined behavior](https://en.cppreference.com/w/cpp/language/ub) due to freeing memory allocated by `new` using `free` instead of `delete`. Get a [good book](https://stackoverflow.com/q/388242/9254539) instead. – eesiraed Jun 05 '19 at 03:57

3 Answers3

2

For reference, here's the piece of code that you modified which you assume is the source of your problems:

Working Code

SinglyLinkedListNode *nnode;
nnode = new SinglyLinkedListNode(data);

Broken Code

SinglyLinkedListNode nnode;
nnode = new SinglyLinkedListNode(data);

Static vs. Dynamic Objects

The difference, as you said is that nnode is a Node pointer object in the working version, whereas in your version, it's a Node object. The problem resides with the use of the new keyword ( see here ).

[The new keyword] creates and initializes objects with dynamic storage duration, that is, objects whose lifetime is not limited by the scope in which they were created.

You're trying to create a static object using dynamic memory. It doesn't work out. You could create it as a static object by just saying:

SinglyLinkedListNode nnode(data);

( see here ) for a more in-depth explanation of instantiating (making an instance) of a class object.

Error Message

 error: no matching function for call to ‘SinglyLinkedListNode::SinglyLinkedListNode()’ SinglyLinkedListNode nnode; ^~~~~ solution.cc:10:9: note: candidate: ‘SinglyLinkedListNode::SinglyLinkedListNode(int)’ SinglyLinkedListNode(int node_data) { ^~~~~~~~~~~~~~~~~~~~ solution.cc:10:9: note: candidate expects 1 argument, 0 provided 

Based on your error message, it seems that it resides more in a problem with your constructors.

It seems to not be able to find a default constructor for your SinglyLinkedListNode class. Of which, you try to call in that function. My guess is that it sees the instantiation of the non-pointer ListNode as a default constructor of that class, where you don't have one. This person had the same problem.

Carson
  • 58
  • 5
2

Can we do node creation without a pointer...

Technically yes, but practically no. Not in this case.

SinglyLinkedListNode nnode;

fails because there is no constructor for SinglyLinkedListNode that takes no parameters.

SinglyLinkedListNode nnode(data);

or

SinglyLinkedListNode nnode{data};

call SinglyLinkedListNode(int node_data) and create a SinglyLinkedListNode, but this SinglyLinkedListNode is a locally scoped Automatic variable. It will be destroyed as soon as the insertNodeAtHead returns. This is useless to you. You'd wind up with what's called a dangling pointer, a pointer to a dead object. Accessing that dead object is Undefined Behaviour. This is often fatal, but believe it or not it's worse when it's not fatal. You don't know what will happen. Often the program looks like it works. And then all of a sudden it doesn't.

You need a dynamic allocation, and the easiest way to hold on to a dynamic allocation is with a pointer.

Side note:

return llist = nnode;

might as well be

return nnode;

the assignment here does nothing. A pointer is the address to an object. When you pass a pointer into a function, the object pointed at is passed by reference, but the function gets a copy of the address. llist = nnode changes the copy of the address, not the original. The calling function has no idea what happens to the copy and carries on with it's original value.

In this case you kind-of lucked out, because

return llist = nnode;

performs the pointless assignment and then returns the value of llist, the value you wanted to return anyway.

user4581301
  • 33,082
  • 7
  • 33
  • 54
0

SinglyLinkedListNode *nnode; nnode = new SinglyLinkedListNode(data);

new returns a pointer. This pointer contains the address of object which means it points to the object(in general language).

SinglyLinkedListNode nnode; nnode = new SinglyLinkedListNode(data);

here nnode is already an object and you are trying to assign it a pointer type.

class_type and class_type* are different. Every pointer for a code which is of any type is of either 4 bytes or 8 bytes depends on compiler since it is an address. So don't confuse with object and object pointer.