Searching and comparing a record

I am still a beginner in C++ and I am always curious about good/best coding methods.

Let's say I have a program which allows a users to edit the salary of a employee.

-
The system will prompt the user which to key in a employee's name first.

-
The system will then check whether or not the username exist.

-
If the username existed, the system will then allow the user to change the employee's
salary.

The salary and the name of the person is stored in a text file.

employeeInfo.txt (formatted by name and salary)

john 1000
mary 2000
bob 3000

user.h

#ifndef user_user_h
#define user_user_h
#include 

class  user  {

public:

    user(std::string userName,std::string salary);

    std::string getUserName();
    std::string getSalary();

    void setUserName(std::string userName);
    void setSalary(std::string salary);

private:
    std::string userName,salary;

};
#endif

user.cpp

#include "user.h"
#include 
#include 
using namespace std;

user::user(string userName,string salary)  {

    setUserName(userName);
    setSalary(salary);

};

string user::getUserName()    {
    return userName;
}
string user::getSalary()  {
    return salary;
}

void user::setUserName(std::string userName) {
    this->userName = userName;
}

void user::setSalary(std::string salary) {
    this->salary = salary;
}

main.cpp

```
#include "user.h"
#include
#include
#include
#include
#include
#include
#include
using namespace std;

int main(){
vector userDetails;
string line;
string userName;
string salary;

ifstream readFile("employeeInfo.txt");

while(getline(readFile,line)) {
stringstream iss(line);
iss >> userName >> salary;

//consturctor
user employeeDetails(userName,
salary
);
userDetails.push_back(employeeDetails);

}
readFile.close();

string name;
cout > name;

for (int i =0; i> newSalary;

Your question is surprisingly hard to answer, largely because you ask one question in your words, but a different question in your code. Your written question is whether looking up a value by sequentially scanning a vector is the best or worst way to do it. And the answer to that question is nuanced. I'll talk more about this in a bit.

However your code shows a full scenario in which you read a file, look up and edit your single user, and display the full vector's contents. In this fuller context, the performance questions of the sequential scan almost definitely do not matter, as reading and displaying the whole vector will cost more time than you could save by having a smarter look-up. A more typical program might let you perform multiple edits, and then the time it took to look up each person might be more important. So that's another improvement to consider.

Scanning a Vector

Scanning a vector with a hand-rolled for loop is not generally considered the best way to do things. It's often much better to find an algorithm that does what you want, and use it. In this case we don't know anything particularly useful about the contents of your vector, so the best bet is going to be find or find_if. To use find, you'll have to overload operator== in something, and pass it around; to use find_if you can instead create a helper function or overload operator(), or in C++11 you can pass a lambda function. Let's examine the middle option:

struct user_by_name {
    user_by_name(string name) : name(name) {}
    bool operator()(user& user) { return user.getUserName() == name; }
    string name;
};

: : :

vector::iterator iterFound = std::find_if(userDetails.begin(), userDetails.end(), user_by_name(name));
if (iterFound != userDetails.end())
{
    : : :
}

The nice thing about this approach is you can create ways to find users by other criteria, such as a user_by_salary, and substitute it in with almost no code change. The downside is that without making other changes, you'll never get better than "linear" performance - on average you'll always have to look through half of the items in your vector to find the one you need.

Scanning faster

If there's a natural ordering for the items in your vector, you have two main options. Both of them require implementing an operator



• You can store the items in a different data structure such as a std::map which makes similar use of the ordering to give you the same performance assistance that lower_bound does on the sorted vector.




Choosing between these cases depends on how the data will be used. Again, with just a single edit in your main program, this is all overkill. But if you are going to have a long-running multi-edit scenario, especially one with thousands of employees being looked up by name and updated, it might be worth examining these options.

Other options

If you were storing a realistic amount of data about each employee, had a realistic number of employees, and had a lot of tasks you wanted to perform on them, chances are you'd find storing their information in a database would make a lot more sense. Databases solve a lot of problems for you; not only do they support fast look-ups, they handle persistence of the data (loading and saving it as necessary) and, for the right scenarios, save you a lot of time and effort. Obviously for learning how to do some of the C++ programming that you show here, using a database may help you less (although learning how to use a database isn't a bad skill either).

Other comments

Finally I wanted to end this review with a list of more generic comments about your actual code. A lot of these things are details you don't need to care about yet, or that may have occurred just in trying to post your code here, but they may help you form better habits as you advance your C++ programming skills.



• using namespace std is frowned upon. It's utter anathema in a header file (which you didn't do), but it's also a risk in your cpp file. You're better off either adding the std:: prefixes, or adding using std::string; using std::vector; etc. However it's unlikely to matter in code like you show here.

• Your use of whitespace is inconsistent. You don't always put spaces after commas, and sometimes use newlines instead. In some places you use more blank lines that I would consider helpful. Also not going to really matter in code this short.

• Your property accessors (getUserName, getSalary) should be const as they should not visibly modify the object, and probably should return a const string& instead of a mutable copy.






• Your header should include instead of , and user.cpp can similarly drop its incldue of ; you don't appear to use anything from iostream in there. Your main.cpp similarly has a lot of unused includes, but I would expect it's more of a testing file so won't harp on it.






• Your search loop will find multiple matches. If you have multiple employees named bruce`, your code w

StackExchange Code Review Q#38629, answer score: 4