Naive password checker I'm looking for advice on how I could improve this. I'm using VS2015 to compile so whatever language features are supported in there is fair game!

Problem: Create a program that reads a username and a password from
the user and grants access if that username and password exists in
code. Bonus points for storing the possible usernames in a file like
so:
user1,pass
user2,pass
...

Encryption is not really a concern; I'm just getting the hang of things. Points I'd like to improve:

I'm using a placeholder vector for the user input, initialized with empty strings. It feels like there would be a more elegant way to get user input without using a holding variable?

I imagine there's a more elegant way to go through each line of the file too, again, lots of placeholder variables for data I could process in place I think.



#include &lt;iostream&gt;
#include &lt;fstream&gt;
#include &lt;vector&gt;
#include &lt;string&gt;

using namespace std;

// Extract the contents of a user file for processing.
vector&lt;string&gt; get_user_list(ifstream&amp; userfile)
{
    string line;
    vector&lt;string&gt; userlist;

    if (userfile.is_open())
    {
        while (getline(userfile, line))
            userlist.push_back(line);

        userfile.close();
    }
    else
        cout &lt;&lt; &quot;Couldn't read from file.&quot;;

    return userlist;
}

// Go through each line of the user list and extract a comma
// seperated username and pass; match against the supplied
// username and pass.
bool check_pass(vector&lt;string&gt; tomatch, vector&lt;string&gt; userlist)
{
    string fuser, fpass;
    bool match = false;

    for (string uline : userlist)
    {
        for (size_t i = 0; i &lt; uline.size(); i++)
        {
            if (uline[i] == ',')
            {
                fuser = uline.substr(0, i);
                fpass = uline.substr(i + 1, string::npos);
                break;
            }
        }

        if (tomatch[0] == fuser &amp;&amp; tomatch[1] == fpass) 
        {
            match = true;
            break;
        }
    }

    return match;
}

int main()
{
    ifstream userfile(&quot;data/users.txt&quot;);
    vector&lt;string&gt; userlist = get_user_list(userfile);
    vector&lt;string&gt; input = { &quot;&quot;, &quot;&quot; };

    cout &lt;&lt; &quot;Enter your username followed by your pass: &quot;;
    cin &gt;&gt; input[0] &gt;&gt; input[1];

    if (check_pass(input, userlist))
        cout &lt;&lt; &quot;ACCESS GRANTED&quot; &lt;&lt; endl;
    else
        cout &lt;&lt; &quot;ACCESS DENIED&quot; &lt;&lt; endl;

    return 0;
}
Whenever you are passing arguments in C++, you must be a bit more careful. Since C++ is a default by-value language, passing in a std::vector the way you are doing it will create a full copy of the vector each time you call the function. This is expensive, and not needed. Instead, you should be passing by const &amp;:

bool check_pass(const vector&lt;string&gt;&amp; tomatch, const vector&lt;string&gt;&amp; userlist)


The same thing applies when using a range-based for loop:

for (const string&amp; uline : userlist)


Or you could simply let C++ deduce the type for you, using auto:

for (const auto&amp; uline : userlist)


string has some methods that can help make your code a bit shorter and clearer; such as find:

auto place = uline.find(',');


As @janos has already mentioned, using a map (or unordered_map) would be a better fit here. This could look something like:

std::unordered_map&lt;std::string, std::string&gt; user_to_pwd;
while (getline(userfile, line)) {
    const auto index = line.find(',');
    user_to_pwd.insert(
        std::make_pair(line.substr(0, index), line.substr(index + 1));    
}


Your check_pass function could then simply take three parameters: the map, and the username and password. This then becomes a (fast) lookup:

bool check_pass(
    const std::unordered_map&lt;std::string, std::string&gt;&amp; user_pwds, 
    const std::string&amp; user, 
    const std::string&amp; password)
{
    auto it = user_pwds.find(user);
    if(it == user_pwds.end()) {
        return false;
    }
    return it-&gt;second == password;
}


Note also that you should not use using namespace std; up the top. This pulls everything from the std namespace into the global namespace, which can cause all sorts of issues. For small programs it probably won't, but it's best not to get into the habit of using it anyway.