sorting for strings (putting a list in alphabetical order)

Pages: 12
So my program reads names from a file and then sorts them by first name.My program does this,but the last name does not follow with the first name.
eg mike orb --- bob orb
bob cats --- mike cats
here is my code;

#include<iostream>
#include<string>
#include<fstream>
using namespace std;


struct persons{
string first_name;
string last_name;
};

void openfile(persons student[], int& item,ifstream&file);
void swap(string& name1, string& name2);
void sortbyfirstname(persons student[], int& item);
void output(persons student[], int& item);
void closefile(ifstream& file);
int main(){
int item;
persons student[1000];
ifstream file;
openfile(student, item,file);
sortbyfirstname(student, item);
output(student, item);
closefile(file);
}

void openfile(persons student[], int& item,ifstream& file){
file.open("students");
if(file.fail()){
cout<<"file open error"<<endl;}
item=0;
while(!file.eof()){
file>>student[item].first_name>>student[item].last_name;
item++;
}
}

void swap(string& name1, string& name2){
string temp;
temp=name1;
name1=name2;
name2=temp;
}

void sortbyfirstname(persons student[], int& item){
for(int a=0; a<item-1; a++)
{
for(int i=0; i<item-1; i++)
if(student[i].first_name > student[i+1].first_name)
swap(student[i].first_name,student[i+1].first_name);


}

}

void output(persons student[], int& item){
for(int i=0; i<item; i++)
cout<< student[i].first_name<<" "<<student[i].last_name<<" "<<endl;
}

void closefile(ifstream& file){
file.close();
cout<<"input file closed"<<endl;
}
Last edited on
How does the file "students" look like? is it just one fullname (ie. first_name <space> last_name) per line?
sortbyfirstname() should be swapping student entries, not just the first name.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
void swap_persons (persons & pers1, persons & pers2)
{   persons temp;
    temp = pers1;
    pers1 = pers2;
    pers2 = temp;
}

void sortbyfirstname (persons student[], int& item)
{   for(int a=0; a<item-1; a++)
    {   for(int i=0; i<item-1; i++)
            if (student[i].first_name > student[i+1].first_name)
                swap_persons (student[i], student[i+1]);
    }
}


Do not loop on ! stream.eof(). This does not work the way you expect. The eof bit is set true only after you make a read attempt on the file. This means after you read the last record of the file, eof is still false. Your attempt to read past the last record sets eof, but you're not checking it there. You proceed as if you had read a good record. This will result in reading an extra (bad) record. The correct way to deal with this is to put the >> operation as the condition in the while statement.
1
2
3
  while (stream >> var) // or while (getline(stream,var))
  {  //  Good operation
  }


PLEASE USE CODE TAGS (the <> formatting button) when posting code.
It makes it easier to read your code and also easier to respond to your post.
http://www.cplusplus.com/articles/jEywvCM9/
Hint: You can edit your post, highlight your code and press the <> formatting button.

Last edited on
sortbyfirstname() should be swapping student entries

agreed and OP: you can do this quite easily by overloading one of the relational operators (> or < ) for struct persons rather than have two different functions swap() and sortbyfirstname(). In your case it would be quite straightforward because the relation operators are already overloaded for the string class that your data members belong to. Similarly overload insertion operator << to print
Thank you guys so much, it works.
Now I want to allow the user to add a name in the file.I have also tweaked my program to have a menu interface, by adding a switch statement. But it is not working.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
#include<iostream>
#include<string>
#include<fstream>
using namespace std;


struct persons{
	string first_name;
	string last_name;
};

void openfile(persons student[], int& item,ifstream&file);
void swappersons(persons& name1, persons& name2);
void sortbyfirstname(persons student[], int& item);
void output(persons student[], int& item);
void closefile(ifstream& file);
void choice();
void sortbylastname(persons student[],int& item);
int main(){
	int item;
	persons student[1000];
	ifstream file;
	openfile(student, item,file);
	choice();
	closefile(file);
}
void choice(){
	int choice,item;
	persons student[1000];
	ifstream file;
	cout<<"please select an option"<<endl;
	cout<<"select 1 to sort names by first name"<<endl;
	cout<<"select 2 to sort names by last name"<<endl;
	cout<<"select 3 to add a name"<<endl;
	cin>>choice;
	switch(choice){
	case 1: sortbyfirstname(student,item);
		    output(student, item);break;
	case 2: sortbylastname(student,item);output(student, item);break;
	case 3: cout<<"option still building"<<endl;break;
	}
}
void openfile(persons student[], int& item,ifstream& file){
	file.open("students");
	if(file.fail()){
			cout<<"file open error"<<endl;}
	item=0;
	while(!file.eof()){
		file>>student[item].first_name>>student[item].last_name;
		item++;
	}
}

void swappersons(persons& name1, persons& name2){
	persons temp;
	temp=name1;
	name1=name2;
	name2=temp;
}

void sortbyfirstname(persons student[], int& item){
	for(int a=0; a<item-1; a++)
	{
		for(int i=0; i<item-1; i++)
			if(student[i].first_name > student[i+1].first_name)
				swappersons(student[i],student[i+1]);


	}

}

void sortbylastname(persons student[],int& item){
	for(int a=0; a<item-1; a++)
		{
			for (int i=0; i<item; i++)
				if(student[i].last_name>student[i+1].last_name)
					swappersons(student[i],student[i+1]);
	}
}
void output(persons student[], int& item){
	for(int i=0; i<item; i++)
		cout<< student[i].first_name<<" "<<student[i].last_name<<" "<<endl;
}

void closefile(ifstream& file){
		file.close();
		cout<<"input file closed"<<endl;
}
 
You switch statement lacks a default case, in-case the user does not input what you expect.
You also never ask for input of variable "item" nor initialize it. So it is sending junk into your functions


As well as something to be said for readability of your program. You'll find it easier to debug if your eyes can follow your code more fluidly. Whitespace has no effect on how the compiler interprets code, and your eyes and brain will appreciate legible code

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
	switch(choice)
	{
	case 1: 
	    sortbyfirstname(student,item);
            output(student, item);
            break;
	case 2:
	    sortbylastname(student,item);output(student, item);
	    break;
	case 3: 
	    cout<<"option still building"<<endl;
	    break;
	 default:
	    cout << "Invalid input, Terminating Program" << endl;
	    break;
	}
Last edited on
I did what you suggested, but my program still is not able to successfully run.
A message occurs saying my program has stopped working.
I actually got it to work, by putting the switch statement in the main. So i'm assuming that you cannot put the switch statement inside a function. But when i'm sorting for last name one entry disappears.
for example,
1
2
3
4
5
6
7
8
9
input
phil apple
joe dance
lenard fish
nick cash
front man
david loop
darn wall
mike ball

darn wall is missing
1
2
3
4
5
6
7
8
output
phil apple 
mike ball 
nick cash 
joe dance 
lenard fish 
david loop 
front man 
Last edited on
Send in the latest version of your program if you need help
Your functions do not work from your "choice" function because you are redeclaring everything, not initializing it nor passing anything meaningful.

Your switch statement works in main because you are initializing the values and passing things to the functions.

You need to walk through your logic to understand why things don't work.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
int main() {
	int item;
	persons student[1000];
	ifstream file;
	openfile(student, item, file);//uses function to open a file by reference
//and fills array
	choice();
	closefile(file);
}
void choice() {
	int choice, item;
	persons student[1000];//declares array of struct persons, uninitialized so it contains garbage
	ifstream file;//redeclare file, does not open or store student names anywhere
	cout << "please select an option" << endl;
	cout << "select 1 to sort names by first name" << endl;
	cout << "select 2 to sort names by last name" << endl;
	cout << "select 3 to add a name" << endl;
	cin >> choice;
	switch (choice) {
	case 1: sortbyfirstname(student, item);//passed an empty array
		output(student, item); break;//passed an empty array
	case 2: sortbylastname(student, item); output(student, item); break;//passed an empty array
	case 3: cout << "option still building" << endl; break;
	}

Last edited on
So this is the most updated version of the program, can someone please explain why one name vanishes when sorting with the last name.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
#include<iostream>
#include<string>
#include<fstream>
using namespace std;


struct persons{
	string first_name;
	string last_name;
};

void openfile(persons student[], int& item,ifstream&file);
void swappersons(persons& name1, persons& name2);
void sortbyfirstname(persons student[], int& item);
void output(persons student[], int& item);
void closefile(ifstream& file);
void sortbylastname(persons student[],int& item);
int main(){
	int item,choice;
	persons student[1000];
	ifstream file;
	openfile(student, item,file);
	cout<<"please select an option"<<endl;
		cout<<"select 1 to sort names by first name"<<endl;
		cout<<"select 2 to sort names by last name"<<endl;
		cout<<"select 3 to add a name"<<endl;
		cin>>choice;
		switch(choice)
		{
		case 1:
		    sortbyfirstname(student,item);
	            output(student, item);
	            break;
		case 2:
		    sortbylastname(student,item);output(student, item);
		    break;
		case 3:
		    cout<<"option still building"<<endl;
		    break;
		 default:
		    cout << "Invalid input, Terminating Program" << endl;
		    break;
		}
	closefile(file);
}

void openfile(persons student[], int& item,ifstream& file){
	file.open("students");
	if(file.fail()){
			cout<<"file open error"<<endl;}
	item=0;
	while(!file.eof()){
		file>>student[item].first_name>>student[item].last_name;
		item++;
	}
}

void swappersons(persons& name1, persons& name2){
	persons temp;
	temp=name1;
	name1=name2;
	name2=temp;
}

void sortbyfirstname(persons student[], int& item){
	for(int a=0; a<item-1; a++)
	{
		for(int i=0; i<item-1; i++)
			if(student[i].first_name > student[i+1].first_name)
				swappersons(student[i],student[i+1]);


	}

}

void sortbylastname(persons student[],int& item){
	for(int a=0; a<item-1; a++)
		{
			for (int i=0; i<item; i++)
				if(student[i].last_name>student[i+1].last_name)
					swappersons(student[i],student[i+1]);
	}
}
void output(persons student[], int& item){
	for(int i=0; i<item; i++)
		cout<< student[i].first_name<<" "<<student[i].last_name<<" "<<endl;
}

void closefile(ifstream& file){
		file.close();
		cout<<"input file closed"<<endl;
}

the names in the file are
1
2
3
4
5
6
7
8
phil apple
joe dance
lenard fish
nick cash
front man
david loop
darn wall
mike ball

the output after last name sort is

1
2
3
4
5
6
7
8
phil apple 
  
mike ball 
nick cash 
joe dance 
lenard fish 
david loop 
front man 

As you can see the name "darn wall" is missing and there is nothing on line 2
The variable item is declared but not initialized or assigned a value at any point in your program yet passed around to several functions - we have no idea of telling what actual value is inside the variable item
Assignment operator is not defined for struct persons, yet various assignment operations are carried out in swappersons (person&, person&) - the compilers default may not be entirely suitable in this case but this might probably the reason why sortbyfirstname() worked
Finally you have a huge array, size 1000, holding only 8 persons objects. I'm not entirely sure if this has any effect or not
why one name vanishes when sorting with the last name.
Compare the code for the two sort functions.
1
2
3
4
5
void sortbyfirstname(persons student[], int& item)
{
    for (int a=0; a<item-1; a++)
    {
        for (int i=0; i<item-1; i++)


1
2
3
4
5
void sortbylastname(persons student[], int& item)
{
    for (int a=0; a<item-1; a++)
    {
        for (int i=0; i<item; i++)

Notice the difference, i<item-1 versus i<item

It is also not a good idea to use eof() as the while loop condition.
In this case, depending on whether there is any whitespace such as a newline '\n' following the last name in the file, the loop may repeat one more time than it should.

Also, there doesn't seem any good reason to pass the ifstream to the function, it isn't carrying any information. Better to avoid the clutter and remove that parameter. Also delete the closefile() function which isn't useful. The file is automatically closed when the fstream object goes out of scope at the end of the function.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
void openfile(persons student[], int& item)
{
    ifstream file("students");
    if (!file)
    {
        cout << "file open error" <<endl;
    }
    
    item = 0;
    while (file >> student[item].first_name >> student[item].last_name)
    {
        item++;
    }
}
line 80 is wrong
for (int i=0; i<item; i++
should be
for (int i=0; i<item-1; i++

Couple of other minor things.
- Testing for eof at input isn't always reliable
- You are doing at least twice as many tests in bubblesort as you need.

Edit: looks like it took me a long time to type! Crossed with @chervil
Last edited on
lastchance wrote:
looks like it took me a long time to type! Crossed with @chervil
It's all good.

Now at least three different people have each advised against testing for eof(), starting with
AbstractionAnon wrote:
Do not loop on ! stream.eof().

maybe the message might be received?
OP: don't loop on eof() ;)
Last edited on
gunnerfunner wrote:
The variable item is declared but not initialized or assigned a value at any point in your program yet passed around to several functions - we have no idea of telling what actual value is inside the variable item

Well, I see your point, though it is assigned a value of zero at line 51:
 
item=0;

Assignment operator is not defined for struct persons, yet various assignment operations are carried out in swappersons (person&, person&) - the compilers default may not be entirely suitable in this case
It is always worth thinking about this. Definitely the default version supplied by the compiler may not always be suitable. In this case it appears ok.

Finally you have a huge array, size 1000, holding only 8 persons objects. I'm not entirely sure if this has any effect or not
It's relatively harmless, the program is only passing pointers around, it should be ok so long as the stack doesn't overflow (someone recently had an array containing 56000 std::strings, I think, which was too large for the usual stack size)
assigned a value of zero at line 51

OP got v lucky with this one, item is assigned a value inside openfile() but since it was declared outside it remained in scope after return and because openfile() came first in main() among all the function calls this value of item could be passed around to the other functions
Thank you everyone for your input. Now just one last thing left, to add a contact I think I know how to ask the user but i don't know how to output it in the file.
Do I make another file or could i store the new names in the program, so the next time you run it by sort first/last name it will include the new name?
1
2
3
4
5
6
void addcontact(persons tmp){
	cout<<"enter first name"<<endl;
	cin>>tmp.first_name;
	cout<<"enter last name"<<endl;
	cin>>tmp.last_name;
}
Last edited on
this is the most recent version for my program
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
#include<iostream>
#include<string>
#include<fstream>
using namespace std;


struct persons{
	string first_name;
	string last_name;
};

void openfile(persons student[], int& item);
void swappersons(persons& name1, persons& name2);
void sortbyfirstname(persons student[], int& item);
void output(persons student[], int& item);
void closefile(ifstream& file);
void sortbylastname(persons student[],int& item);
void addcontact(persons tmp);
int main(){
	int item,choice;
	persons student[1000],tmp;
	ifstream file;
	openfile(student, item);
	while(choice!=4){
	cout<<"MAIN MENU- please select an option"<<endl;
		cout<<"select 1 to sort names by first name"<<endl;
		cout<<"select 2 to sort names by last name"<<endl;
		cout<<"select 3 to add a name"<<endl;
		cout<<"select 4 to end program"<<endl;
		cin>>choice;
		switch(choice)
		{
		case 1:
			cout<<"*****************************"<<endl;
		    sortbyfirstname(student,item);
	            output(student, item);
	            cout<<endl;
	            break;
		case 2:
			cout<<"*****************************"<<endl;
		    sortbylastname(student,item);output(student, item);
		    cout<<endl;
		    break;
		case 3:
		    addcontact(tmp);
		    break;
		case 4:
			cout<<"program has ended"<<endl;
			return 0;
		 default:
		    cout << "Invalid input, Terminating Program" << endl;
		    break;
		}
	closefile(file);
}
}
void openfile(persons student[], int& item)
{
    ifstream file("students");
    if (!file)
    {
        cout << "file open error" <<endl;
    }

    item = 0;
    while (file >> student[item].first_name >> student[item].last_name)
    {
        item++;
    }
}

void swappersons(persons& name1, persons& name2){
	persons temp;
	temp=name1;
	name1=name2;
	name2=temp;
}

void sortbyfirstname(persons student[], int& item){
	for(int a=0; a<item-1; a++)
	{
		for(int i=0; i<item-1; i++)
			if(student[i].first_name > student[i+1].first_name)
				swappersons(student[i],student[i+1]);


	}

}

void sortbylastname(persons student[],int& item){
	for(int a=0; a<item-1; a++)
		{
			for (int i=0; i<item-1; i++)
				if(student[i].last_name>student[i+1].last_name)
					swappersons(student[i],student[i+1]);
	}
}

void addcontact(persons tmp){
	cout<<"enter first name"<<endl;
	cin>>tmp.first_name;
	cout<<"enter last name"<<endl;
	cin>>tmp.last_name;
}

void output(persons student[], int& item){
	for(int i=0; i<item; i++)
		cout<< student[i].first_name<<" "<<student[i].last_name<<" "<<endl;
}

void closefile(ifstream& file){
		file.close();
		cout<<"********************************"<<endl;
}

I just can't figure out how to store the user input "new name" in the file for future sorting.
Pages: 12