Hello coder0101,
After working on your code and figuring out the sort function this is what I have come up with.
As a start you need more header files than just "iostream". Your IDE may be letting you define
string name
, from the struct, but when you get to a "cout" statement the tries to pint the string it does not know what to do without the "string" class defined in the "string" header file. Also "std::getline" will generate an error as being undefined.
I find that these includes tend to be needed by most programs:
1 2 3 4 5 6 7 8
|
#include <iostream>
//#include <iomanip> // <--- For "std::fixed", "std::setprecision()" and "std::setw()" + others.
#include <limits>
#include <string> // <--- Missing "<string>".
#include <vector>
#include <cctype> // <--- For "std::tolower() and std::toupper()" + others.
using namespace std; // <--- Best not to use.
|
Also the using statement normally comes after all the includes not in between, but really makes no difference, just looks better. But as the comment says you should avoid this using statement. Better to learn how to qualify what is in the "std" name space now then all at once later.
For your struct:
1 2 3 4 5 6 7 8 9
|
struct Contact
{
string sName;
string sPhone;
string sEmail;
Contact() {}
Contact(string name, string phone, string email) : sName(name), sPhone(phone), sEmail(email) {}
};
|
Starting the name of the struct is not mandatory, but very helpful. This also applies to classes.
Defining the variable starting with an "s" is not necessary, but also can be very helpful as demonstrated in the overloaded ctor.
Based on
jlb's suggestion I used the initialization list, but either way works. This also demonstrates how a good variable name, something more than a single letter, can make the code easier to read and understand.
Notice there is no (;) after the {}s.
In the "displayContacts" function you do not need a "cout" statement for every line. That is what the insertation operator, (<<), is for. The code would be:
1 2 3 4 5 6 7 8 9 10 11 12 13
|
void displayContacts(const vector<Contact>& mycontacts) // <--- Changed. Added the "const".
{
std::cout << '\n';
for (size_t i = 0; i < mycontacts.size(); i++)
{
cout
<< " Contact " << i + 1 << '\n'
<< " Name: " << mycontacts[i].sName << '\n'
<< " Phone: " << mycontacts[i].sPhone << '\n'
<< " Email: " << mycontacts[i].sEmail << "\n\n";
}
}
|
I added the "const" here because the vector is better passed by reference. The "const" means you can use the the vector, but can not accidentally change anything.
This also has the advantage of the code looking more like what is displayed on the screen. To that end adding an extra space before "name" lines up the (:)s for a better looking display. Adding line 3 means that your output will not all run together line after line.
The "sort" function is the biggest problem. Consider calling the function "SortVec". Not only is it more descriptive, but less chance of it being confused with "std::sort". Since you are using the line:
using namespace std;
the compiler will look for "std::sort" first. If it finds it you could have a problem.
Inside your function you have:
1 2 3 4 5 6 7 8 9 10 11 12 13 14
|
int a = mycontacts.size();
string added = mycontacts[a].name;
for (int i = 0; i < mycontacts.size(); i++)
{
string first = mycontacts[i].name;
for (int j = i + 1; i < mycontacts.size(); j++)
{
if (mycontacts[j].name < first)
{
string first = mycontacts[j].name;
mycontacts[i] = mycontacts[j];
mycontacts[j] = first;
}
}
|
Lines 1 and 2 make no sense. And line 2 is causing the program to crash because the ".size" function returns the number of elements in the vector. The returned number is of type "size_t" which is an "unsigned implementation-defined" integer type. Depending on your header files this could be an "int" or a "long". See
https://en.cppreference.com/w/c/types/size_t
This means that
mycontacts[a]
the value of "a" is 1 past the end of the vector.
In your for loops, since the ".size" function returns a "size_t", both "i" and "j" should be defined as "size_t". This will eliminate the warnings from the compiler, but even if you do not change it the "int" will still work.
In the outer loop you do not want
i < mycontacts.size()
. This would end up making "j" start 1 past the end of the vector. The condition on the inner for loop is fine.
Inside the inner loop the if statement should be making use of both "i" and "j" since "i" would start at element (0)zero and "j" would start at element (1). Using that to compare 2 names is what you want.
Inside the if statement you want to swap complete objects. Line 10 has no place or use in the part of the code. And I believe you changed line 12 from what you originally posted because I remember line 12 trying to set an object to a "std::string", but you code no longer reflects this.
If you have changes to your code put them in another post. Otherwise your original post will look as if nothing is wrong then it makes it hard to understand what the thread is about.
You did make the proper correction to line 12.
Your sort should start something like this:
1 2 3 4 5 6 7 8 9 10 11 12 13
|
void SortVec(vector<Contact>& mycontacts) // <--- Notice the space after the &.
{
//int a = mycontacts.size() - 1;
//string added = mycontacts[a].sName; // <--- A is the total elements of the vector. "mycontacts[a]" is 1 past the end of the vector.
for (size_t i = 0; i < mycontacts.size() - 1; i++)
{
Contact first;
for (size_t j = i + 1; j < mycontacts.size(); j++) // <--- Changed. "i < mycontacts"
{
if (mycontacts[j].sName < mycontacts[i].sName)
|
The commented lines are never used in the function.
Then inside the if statement you need to deal with objects not a combination of objects and "std::string"s.
In "addContact" it actually works down to
mycontacts.resize(mycontacts.size() + 1);
. Resizing the vector will make it 1 larger, but that last element will be an empty struct then the "push_back" will add 1 more to the end. Not what you want.
I ended up with this:
1 2 3 4
|
//Adding Contact alphabetically
//mycontacts.resize(mycontacts.size() + 1);
//mycontacts.push_back(Contact(name, phone, email));
mycontacts.emplace_back(name, phone, email); // <--- Will construct an object of the struct and add it to the vector.
|
The only other thing I would suggest is after you enter a name you should check that each element of the string is converted to a lower case letter. Otherwise the sort may not give you what you want.
In the "menu" function I added 1 line of code:
1 2 3
|
cin >> choice;
choice = static_cast<char>(std::toupper(static_cast<unsigned char>(choice))); // <--- Added.
|
This way when you return to "main" you will have a capital letter. to work with.
In "main" instead of the if/else if statements, BTW you are missing a final else to deal with any letter that does not match, a switch would be a better choice here.
Andy