Code Not Working (Sorting Names) in C++

Please tell me why my CODE is not working and help me to use getline if you have no idea of my problem.

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
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
#include <iostream>
#include <string>
#include <fstream>

using namespace std;

size_t readNames(string filename, string surnames[], string firstnames[]);
void sortSurnameFirst (string [], string[], size_t);
void displayNames (string [], string[], size_t);
void sortFirstNames (string [], string [], size_t);

int main()
{
int const SIZE = 1000;
size_t count = 0;
int choice;

string names[SIZE];
string surnames[SIZE];

ifstream inputFile;
    string filename;
    int number;

    cout << "Enter the file name: ";
    cin >> filename;

    inputFile.open(filename.c_str());

if(inputFile)
{

while(count <SIZE && inputFile>>surnames[count] >> names[count])
{
count ++;
}

inputFile.close();

cout << " 1. Sort names by surnames then first names "<<endl;
cout << " 2. Sort names by first names then by surnames "<<endl;
cout<< "Enter your sorting choice: " ;
cin >> choice;

if (choice == 1)
{
cout<< "The names are sorted by surnames then first names"<<endl;
sortSurnameFirst (surnames, names,count);
displayNames(surnames, names,count);
}

if (choice == 2)
{
cout<< "The names are sorted by first names then surnames"<<endl;
sortSurnameFirst (surnames, names,count);
displayNames(surnames, names,count);
}

}

else
{
cout << "The following file doesnt exist" << endl;
}



return 0;
}






void sortSurnameFirst (string surnames[], string names[], size_t size)
{
bool swap;
string sort ;
string temp;
do
{
swap = false;
for (size_t i =0; i < size ; i++)
{
if (surnames[i] > surnames[i++])
{
sort = surnames[i];
surnames[i] = surnames[i++];
surnames[i++] = sort ;
swap = true;


}


if ( surnames[i] == surnames[i++] )
{
if (names[i] > names[i++] )
{
temp= names[i];
names[i] = names[i++];
names[i++] = temp ;
sort = surnames[i];
surnames[i] = surnames[i++];
surnames[i++] = sort ;

swap = true;
}
}



}

}
while (swap);
}


void displayNames( string surnames[], string names[], size_t size )
{

for ( size_t i =0; i < (size - 1) ; i++)
{
cout << surnames[i]<<" " << names[i] << endl;
}
}


void sortFirstNames (string surnames[], string names[], size_t size)
{
bool swap;
string sort1, temp1;
do
{
swap = false;

for ( size_t i =0; i < size ; i++)
{
if ( names[i] >names[i++] )
{
sort1 = names[i];
names[i] = names[i++];
names[i++] = sort1 ;
swap = true;

}
if ( names[i] == names[i++] )
{
if (surnames[i] > surnames[i++] )
{
temp1 = surnames[i];
surnames[i] = surnames[i++];
surnames[i++] = temp1 ;
sort1 = names[i];
names[i] = names[i++];
names[i++] = sort1 ;
swap = true;
}
}



}

}while (swap);
}
  
i++ increments i (it actually changes the value contained in i) and evaluates to the original value. This means you're advancing through your arrays much faster than you should be - all your indices are broken.

To swap the values of two variables, write a swap function of your own, or use the built-in swap std::swap().
Last edited on
Can you please be specific on which part (or which line(s)) must I really change and what must I replace? That would be helpful please...
Last edited on
It's the if statements where you're comparing the current name/surname to the next one in the list. Currently, you're using the ++ increment operator on lines 141 and 151. The i++ expression actually increments the current value of 'i' by 1, but returns the ORIGINAL value of i before the increment took place.

So, for example if 'i' is 1, and you do this:
if(names[i] == names[i++])

it translates into this:

if(names[1] == names[1]) //however, i has been incremented to 2

So, as mbozzi pointed out, that is messing up your entire logic. Fix those parts first and see what happens...

Thanks!
Joe
sparkprogrammer@gmail.com

Still, is not working. You can copy and paste it and check if it works.
#include <iostream>
#include <string>
#include <fstream>

using namespace std;

size_t readNames(string filename, string surnames[], string firstnames[]);
void sortSurnameFirst (string [], string[], size_t);
void displayNames (string [], string[], size_t);
void sortFirstNames (string [], string [], size_t);

int main()
{
int const SIZE = 1000;
size_t count = 0;
int choice;

string names[SIZE];
string surnames[SIZE];

ifstream inputFile;
string filename;
int number;

cout << "Enter the file name: ";
cin >> filename;

inputFile.open(filename.c_str());

if(inputFile)
{

while(count <SIZE && inputFile>>surnames[count] >> names[count])
{
count ++;
}

inputFile.close();

cout << " 1. Sort names by surnames then first names "<<endl;
cout << " 2. Sort names by first names then by surnames "<<endl;
cout<< "Enter your sorting choice: " ;
cin >> choice;

if (choice == 1)
{
cout<< "The names are sorted by surnames then first names"<<endl;
sortSurnameFirst (surnames, names,count);
displayNames(surnames, names,count);
}

if (choice == 2)
{
cout<< "The names are sorted by first names then surnames"<<endl;
sortSurnameFirst (surnames, names,count);
displayNames(surnames, names,count);
}

}

else
{
cout << "The following file doesnt exist" << endl;
}



return 0;
}






void sortSurnameFirst (string surnames[], string names[], size_t size)
{
bool swap;
string sort ;
string temp;
do
{
swap = false;
for (size_t i =0; i < size ; i++)
{
if (surnames[i] > surnames[i++])
{
sort = surnames[i];
surnames[i] = surnames[i++];
surnames[i++] = sort ;
swap = true;


}


if ( surnames[i] == surnames[i++] )
{
if (names[i] > names[i++] )
{
temp= names[i];
names[i] = names[i++];
names[i++] = temp ;
sort = surnames[i];
surnames[i] = surnames[i++];
surnames[i++] = sort ;

swap = true;
}
}



}

}
while (swap);
}


void displayNames( string surnames[], string names[], size_t size )
{

for ( size_t i =0; i < (size - 1) ; i++)
{
cout << surnames[i]<<" " << names[i] << endl;
}
}


void sortFirstNames (string surnames[], string names[], size_t size)
{
bool swap;
string sort1, temp1;
do
{
swap = false;

for ( size_t i =0; i < size ; i++)
{
if ( names[i] >names[i++] )
{
sort1 = names[i];
names[i] = names[i++];
names[i++] = sort1 ;
swap = true;

}
if ( names[i] == names[i++] )
{
if (surnames[i] > surnames[i++] )
{
temp1 = surnames[i];
surnames[i] = surnames[i++];
surnames[i++] = temp1 ;
sort1 = names[i];
names[i] = names[i++];
names[i++] = sort1 ;
swap = true;
}
}



}

}while (swap);
}
You can copy and paste to see if is working or not, it is really not working at all.
I'll copy and paste this later, but from what I see, your code hasn't changed?

The if statements still use the post-increment operator ++...
Please copy and paste it ASAP if you can. I just don't understand you on what and how I must change. I tried many ways, that's why I even send you the initial code because when I change I get syntax errors a lot. So, please show me.
uh...why are you not trying it yourself to see if it works ???(it's better to check and find answer for a problem first without asking others and if that failed, ask other)

so to summarize..why dont you find the problem and try to fix it.... if it failed, then you can ask
:)
besides rather than
 
if (surnames[i] > surnames[i++]) //this changes i 


it should be
 
if (surnames[i] > surnames[i + 1]) //this doesn't 
I tried everything, even your solution Flaze... It didn't work. You can try to run it and see.
I tried everything, even your solution Flaze... It didn't work. You can try to run it and see.

Of course, making one single change is not going to make it "work" since you have the same problem in multiple places. Get rid of increments where they don't logically make sense.
But how? Please guys help me. :(
consider using a struct Person with surname and firstname as it's data-members. The overload operator < to sort Person instances either by surname or firstname (you can also use function objects, lambdas) - a recent example can be found here:
http://www.cplusplus.com/forum/beginner/214667/#msg999098
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
 
#include <iostream>
#include <string>
#include <vector>
#include <algorithm>

struct Song
{
    std::string m_title;
    std::string m_artist;
    double m_duration;

    Song(const std::string& title, const std::string& artist, const double duration)
        : m_title(title), m_artist (artist), m_duration(duration){}
    //http://en.cppreference.com/w/cpp/language/initializer_list
    bool operator < (const Song& rhs)
    //for sorting
    {
        return m_title < rhs.m_title;
    }
};
std::ostream& operator << (std::ostream& os, const Song& s)
//for printing
{
    os << s.m_title << " by " << s.m_artist << ". Running time: " << s.m_duration << " mins \n";
    return os;
}
int main()
{
    std::vector<Song> mySongs{};
    std::cout << "How many songs would you like to add: \n";
    size_t num{};
    std::cin >> num;
    std::cin.ignore();//ignores newline character in the input stream
    for (auto i = 0; i < num; ++i)
    {
        std::cout << "Enter title of song# " << i + 1 << ": \n";
        std::string title{};
        getline(std::cin, title);
        std::cout << "Enter artist for this song: \n";
        std::string artist{};
        getline(std::cin, artist);
        std::cout << "Enter duration (in mins) for this song: \n";
        double duration{};
        std::cin >> duration;
        std::cin.ignore();
        mySongs.emplace_back(Song(title, artist, duration));
        //http://en.cppreference.com/w/cpp/container/vector/emplace_back
    }
    std::sort(mySongs.begin(), mySongs.end());
    std::cout << "\nSorted (by title) song-list: \n\n";
    for (const auto& elem : mySongs)std::cout << elem;
}
 
Did you run the code and take a look at the results? Always be sure you've done that before posting the entire program here. This way, it'll be easier for us to help you and also for you to understand what we're talking about.

Joe
Topic archived. No new replies allowed.