patterncppMinor
Contact search application
Viewed 0 times
contactsearchapplication
Problem
Create a C++ program which will act like contact search app in mobile.
For example, if the following contacts are saved in contact list:
Massi
Pradip
Prasad
Praveed
Raju
And if user type "a" then following names should be displayed:
Massi
Pradip
Prasad
Praveed
Raju
if user type "Pr" then following names should be displayed:
Pradip
Prasad
Praveed
Basically it should display all the names which contain the search
string anywhere in name.
For example, if the following contacts are saved in contact list:
Massi
Pradip
Prasad
Praveed
Raju
And if user type "a" then following names should be displayed:
Massi
Pradip
Prasad
Praveed
Raju
if user type "Pr" then following names should be displayed:
Pradip
Prasad
Praveed
Basically it should display all the names which contain the search
string anywhere in name.
#include
#include
#include
#include
#include
#include
using namespace std;
int main() {
map contacts;
contacts["Pradip"]=999092456;
contacts["Raju"]=999092457;
contacts["Praveed"]=999092445;
contacts["Prasad"]=999092415;
contacts["Asim"]=999092785;
contacts["Massi"]=999092215;
int key_code;
string key;
while (true)
{
if ( kbhit() ){
key_code = getch();
system("CLS");
if (key_code == 8 ){
key.size() ? key.pop_back(): 0;
}
else{
key+=key_code;
}
cout::iterator &tter = contacts.begin();tter != contacts.end();tter++)
{
if (string::npos == tter->first.find(key) && !key.empty()){
continue;
}
std::coutfirst.c_str()second<<std::endl;
}
}
else
continue;
}
return 0;
}Solution
Here are some observations that may help you improve your code.
Don't abuse
Putting
Don't use
There are two reasons not to use
Isolate platform-specific code
In this code, there are several things that are DOS/Windows only including
Eliminate "magic numbers"
There are a few numbers in the code, such as
Don't abuse the ternary operator
The code currently includes this inscrutable bit of code:
This is not valid C++ even if your compiler accepts it. The problem is that the two options must both evaluate to the same type.
Be careful with references
The
That's not right because
Or even better, you could use the next suggestion if your compiler is compliant.
Use "range-for" to simplify your code
Instead of using an explicit iterator, your for loop could use "range-for":
Simplify I/O by defining a function
This code outputs each contact when needed but it does so using a relatively long formatting string. I'd suggest creating a function do that so that that formatting code is isolated to one place. In particular, I'd create an
Note, too, that I've used simply
Use an initializer list to initialize static data
Use of an initializer list simplifies construction of data structures:
Avoid
Looping constructs in code are generally considerably easier to read and understand if they do not use
Think about the user
There is no obvious way to gracefully exit the program. I'd suggest using a
Omit
When a C++ program reaches the end of
Don't abuse
using namespace stdPutting
using namespace std at the top of every program is a bad habit that you'd do well to avoid. Don't use
system("cls")There are two reasons not to use
system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a separate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. For example, if your terminal supports ANSI Escape sequences, you could use this:void cls()
{
std::cout << "\x1b[2J";
}Isolate platform-specific code
In this code, there are several things that are DOS/Windows only including
#include and the getch() and kbhit() functions within that, and also system("cls"); that I've already mentioned. Your code runs successfully on Linux if I supply those missing functions, but it would be nice if there were an #ifdef WINDOWS already in the code so that one could recompile without having to alter the source code.Eliminate "magic numbers"
There are a few numbers in the code, such as
8 and 14 that have a specific meaning in their particular context. By using named constants such as BACKSPACE or COLUMN_WIDTH, the program becomes easier to read and maintain. Don't abuse the ternary operator
The code currently includes this inscrutable bit of code:
key.size() ? key.pop_back(): 0;This is not valid C++ even if your compiler accepts it. The problem is that the two options must both evaluate to the same type.
std::string.pop_back() returns void but 0 is an int. The fix is to simply use an if instead which makes the code both valid and readable C++:if (key.size()) {
key.pop_back();
}Be careful with references
The
for loop starts with this:map::iterator &tter = contacts.begin();That's not right because
tter should be an iterator, not a reference to an iterator. It should instead be written as:map::iterator tter = contacts.begin();Or even better, you could use the next suggestion if your compiler is compliant.
Use "range-for" to simplify your code
Instead of using an explicit iterator, your for loop could use "range-for":
for (const auto &person : contacts)Simplify I/O by defining a function
This code outputs each contact when needed but it does so using a relatively long formatting string. I'd suggest creating a function do that so that that formatting code is isolated to one place. In particular, I'd create an
operator<< function like so:std::ostream& operator &p) {
return out
<< std::setw(4) << "Name : " << std::setw(10) << std::left << p.first
<< std::setw(14) << ", Number : " << std::setw(10) << p.second;
}Note, too, that I've used simply
p.first rather than p.first.c_str().Use an initializer list to initialize static data
Use of an initializer list simplifies construction of data structures:
std::map contacts{
{"Pradip",999092456},
{"Raju",999092457},
{"Praveed",999092445},
{"Prasad",999092415},
{"Asim",999092785},
{"Massi",999092215}
};Avoid
continue and breakLooping constructs in code are generally considerably easier to read and understand if they do not use
continue or break. In this case, it's very easy to avoid them:for (const auto &person : contacts) {
if (string::npos != person.first.find(key)) {
std::cout << person << std::endl;
}
}Think about the user
There is no obvious way to gracefully exit the program. I'd suggest using a
switch to control when the program is done. Something like this:bool done=false;
while (!done)
{
if ( kbhit() ){
key_code = getch();
cls();
switch (key_code) {
case BACKSPACE:
if (key.size()) {
key.pop_back();
}
break;
case CTRL_C:
done = true;
break;
default:
key+=key_code;
}
std::cout << "Contacts with : " << key << std::endl;
// etc.Omit
return 0When a C++ program reaches the end of
main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main.Code Snippets
void cls()
{
std::cout << "\x1b[2J";
}key.size() ? key.pop_back(): 0;if (key.size()) {
key.pop_back();
}map<string,int>::iterator &tter = contacts.begin();map<string,int>::iterator tter = contacts.begin();Context
StackExchange Code Review Q#96656, answer score: 7
Revisions (0)
No revisions yet.