fstream is writing on file against an if else statement

Question

I have a file named "password.txt" which has 2 rows of usernames and passwords

2 string vectors named, user & password

This function uses a new username and password and checks if the username already exists in the user vector using a for loop

If it does exist, it breaks out of the loop If it does not exist, it will add the username and password to the vectors and write them both on the file

void PasswordFile::addpw(string newuser, string newpassword) {
    fstream file;
    file.open("password.txt", ios::app);
    
    //if the vectors is empty it automatically adds a new user
    //is working
    if (user.size() < 1){
        user.push_back(newuser);
        password.push_back(newpassword);
        file << newuser << " " << newpassword << "\r";
    }

    //the loop (is not working)

    for (int i=0; i < user.size(); i++){
        string name = user[i];
        if (name == newuser) {
            break;
        } //it shouldn't continue past this point if the name is in the vector but it does

        else {
            cout << newuser;
            cout << " is being written onto the file\n";
            user.push_back(newuser);
            password.push_back(newpassword);
            file << newuser << " " << newpassword << "\r";
        }
    }
    
    file.close();
}

the problem is that it is writing the second name twice and the third name three times

I know also that it is pushing the newuser and newpassword multiple times onto the vector, but I can't figure out where or why its looping more than it should or why its ignoring the break and writing onto the file anyway

"password.txt" 1 run

dbotting 123qwe
egomez qwerty
tongyu liberty
tongyu liberty

"password.txt" second run

dbotting 123qwe
egomez qwerty
tongyu liberty
tongyu liberty
egomez qwerty
tongyu liberty
tongyu liberty

it keeps adding the last 2 every time its run

Answer 1

Remy has done a good job of pointing out the problems you're seeing.

Rather than fixing your loop, I would fix the data structures you're using.

Instead of a pair of vectors, one for user names and one for passwords, I'd start by creating an std::map to hold the usernames and passwords:

std::map<std::string, std::string> credentials;

A map already provides most of what you want, as far as guaranteeing that an entry is unique (each username occurs only once), and lets you check whether an insertion succeeded, so an equivalent of your addpw comes down to something like this:

std::map<std::string, std::string> credentials;

void addpw(std::string newuser, std::string newpass) {
    if (credentials.insert({newuser, newpass}).second) {
        std::cout << "Adding new user: " << newuser << "\n";
        file << newuser << " " << newpass << "\n";
    } else {
        std::cerr << "User already exists--not adding\n";
    }
}

Of course, if you care about security at all, you really don't want to store user names and passwords in a plain text file. In fact, you usually don't want to store actual passwords at all--as a bare minimum, you want to salt the password, the hash it with a cryptographic hash algorithm (e.g., SHA-256) then store the salt and the hash, not the password itself. Then to check a user's credentials when they try to log in, you salt what they provide, hash it, and compare the resulting hash to the one you have on file and see if they match.

Now don't get me wrong: that's not the ideal to provide truly maximum security--but at least it's enough to make hackers work a little bit to steal your users' credentials.