Starting a for() loop in case of a linked list from Nth node?

[vboro]

Heyy!

When creating a linked list from a struct array of countries, I have come across the problem with the first element. Everything is sorted in a growing order by the population number, except the first element, to which I should resemble other elements to. Do you have any idea what can I do wrong?

Thanks for the replies in advance!!

The code:

#include<iostream>
#include<string>

struct country_t{
	
	std::string name;
	double quantity;
	
};

struct Node{
	
	std::string name;
	double num;
	Node* next;
	
};

Node* insert(Node* head, country_t element);
void print(Node* head);

int main(){
	
	country_t arr[] = {
		
		{"Hungary", 10},
		{"Serbia", 6.95},
		{"Czeh Republic", 10.65},
		{"Romania", 19.41},
		{"England", 55.98},
		{"Austria", 8.86}
		
	};
	
	int n = sizeof(arr) / sizeof(arr[0]);
	
	Node* head = nullptr;
	
	for(int i = 0; i < n; i++){
		head = insert(head, arr[i]);
	}
	
	print(head);
	
	return 0;
}

void print(Node* head){
	
	Node* it = head;
	
	while(it->next != nullptr){
		std::cout << "Country: " << it->name << ", population: " << it->num << " million\n";
		it = it->next;
	}
	
}

Node* createNode(country_t element){
	
	Node* new_node = new Node;
	if(new_node){
		new_node->name = element.name;
		new_node->num = element.quantity;
		new_node->next = nullptr; 
	}
	
	return new_node;
}

Node* insert(Node* head, country_t element){
	
	Node* prev = head;
	Node* new_node = createNode(element);
	
	if(head == nullptr){
		head = new_node;
		
	}else if(head != nullptr){
		
		for(Node* i = head; i->num < new_node->num && i->next != nullptr; i = i->next){
			prev = i;
			
		}
		new_node->next = prev->next;
		prev->next = new_node;
	}
	
	return head; 
}

The result:

Country: Hungary, population: 10 million
Country: Austria, population: 8.86 million
Country: Czeh Republic, population: 10.65 million
Country: Romania, population: 19.41 million
Country: England, population: 55.98 million

[Ganado]

Do you have any idea what can I do wrong?

There's an infinite number of things you can do wrong. The world is your oyster! :)

Within your "(head != nullptr)" branch in your insert function, you never allow for the opportunity for the head pointer to be updated.

The quick fix way would be to add another branch for if head != nullptr && (number being inserted < head's number).

There is a more eloquent solution one could find, using a pointer-to-pointer or reference-to-pointer. Perhaps if you wanted a challenge, you could explore this.

Some other stuff:

In the code you have shown,

if(head == nullptr){

}else if(head != nullptr){

}

That "else if" can just be an "else".

______________________________

	Node* it = head;
	
	while(it->next != nullptr){

What happens here if head is null? What happens here if head is not null, but the next element is null? Will anything be printed? It seems you will always not print one of the nodes.

______________________________

	Node* new_node = new Node;
	if(new_node){

This check is redundant. If the call to new were to fail, it would throw a bad allocation exception.

______________________________

Fix, part 1:
Change your printing logic:

	
	Node* it = head;
	
	while(it != nullptr){
		std::cout << "Country: " << it->name << ", population: " << it->num << " million\n";
		it = it->next;
	}
	

Fix, part 2.
< Edit: I removed it, because there is still a bug in the else part of the code, so my solution was wrong. Just look at dutch's code >

[dutch]

#include <iostream>
#include <iomanip>
#include <string>

struct Country {
    std::string name;
    double      pop;
};

struct Node {
    Country country;
    Node*   next;
    Node(Country c, Node* n = nullptr) : country(c), next(n) { }
};

void listInsert(Node*& head, const Country& data);
void listPrint(const Node* head);
void listDelete(Node*& head);

int main() {
    Country countries[] = {
        {"Hungary",       10.00},
        {"Serbia",         6.95},
        {"Czeh Republic", 10.65},
        {"Romania",       19.41},
        {"England",       55.98},
        {"Austria",        8.86}
    };
    int n = sizeof countries / sizeof countries[0];

    Node* head = nullptr;
    for (int i = 0; i < n; i++)
        listInsert(head, countries[i]);
    listPrint(head);
    listDelete(head);
}

void listDelete(Node*& head) {
    while (head) {
         auto del = head;
         head = head->next;
         delete del;
    }
}

void listPrint(const Node* head) {
    using namespace std;
    cout << fixed << setprecision(2);
    for (const Node* nd = head; nd; nd = nd->next)
        cout << left  << setw(15) << nd->country.name << "  "
             << right << setw( 6) << nd->country.pop
             << " million\n";
}

void listInsert(Node*& head, const Country& data) {
    Node *nd = head, *prev = nullptr;
    for ( ; nd && nd->country.pop < data.pop; nd = nd->next)
        prev = nd;
    (prev ? prev->next : head) = new Node(data, nd);
}

[vboro]

It worked!! I'm soo happy you can't imagine ^^ :D Thanks a bunch for the help!!

[seeplus]

See also http://www.cplusplus.com/forum/beginner/278817/ with working solutions.