假银行账户类

Question

我只是对我如何使它更有效率或更干净，以及我应该修复的总体上的一些评论。

class bankAcc
{
public:
    bankAcc() = default;
    bankAcc(string fn, string ln) : firstName(fn), lastName(ln) {}
    bool createUser(istream &is);
    bool login() const;
private:
    bool correctLogin(const string&, const string&) const;
    static vector<string> usernames;
    static vector<string> passwords;
    string firstName, lastName;
    string username, password;
};

vector<string> bankAcc::usernames;
vector<string> bankAcc::passwords;

bool bankAcc::createUser(istream &is) {
    is >> username >> password;
    if (is.fail()) {
        cout << "Invalid option" << endl;
        cin.clear();
        return false;
    }
    usernames.push_back(username);
    passwords.push_back(password);
    return true;
}

bool bankAcc::login() const {
    bool login;
    string tempUser, tempPass; 
    cin >> tempUser >> tempPass;
    if (cin.fail()) {
        cout << "Invalid options" << endl;
        cin.clear();
        return false;
    }
    login = correctLogin(tempUser, tempPass);
    if (login == true) {
        cout << "Logged on" << endl;
        return login;
    }   
    else {
        cout << "Incorrect info" << endl;
        return login;
    }
    return login;
    }

bool bankAcc::correctLogin(const string &tempUser, const string &tempPass) const {

    if (usernames.empty() || passwords.empty())
        return false;
    bool usernamePassed, passwordPassed; int cnt = 0;
    for (auto c : usernames) {
        if (tempUser == c) {
            usernamePassed = true;
            break;
        }
        else {
            usernamePassed = false;
        }
        ++cnt;
    }
    if (passwords[cnt] == tempPass && cnt < passwords.size()) {
        passwordPassed = true;
    }
    else {
        passwordPassed = false;
    }
    return usernamePassed && passwordPassed;
}

Answer 1

我有几个建议：

在构造函数中，当完全没有必要时，您将通过复制传递string。您应该通过const引用传递这些内容：

bankAcc(const string &fn, const string &ln) { /*...*/ }

此外，这个构造函数应该检查fn或ln是否是干净的输入(即它们是有效的姓和名)。你可以这样做：

bankAcc(const string &fn, const string &ln) 
{
    if (fn == "" || ln == "")
        throw std::invalid_argument("Can't have empty names");
    firstName = fn;
    lastName = ln;
}

说到firstName和lastName，我在这里根本看不出它们有什么用处，因为它们只在构造函数中使用，没有其他地方使用。需要他们吗？

您还应该在这里练习Principle of Sole Responsibility。换句话说，每个类只应该有一个单独的责任来照顾。您的类名为bankAcc，但在我看来，它更像是一个BankAccountLoginValidator类。你的班级没有做银行账户应该做的任何事情(没有余额，没有取款/存款计划等)。此外，您通常应该将I/O操作与该类中的验证代码分开。让类对象的调用者执行I/O，然后让它们使用您的类来验证它们的输入。你的班级不应该对I/O负责。

最后，使用两个向量来存储用户名和密码是效率低下的，因为您最终会花费O(n)时间在usernames向量中搜索正确的用户名。相反，可以使用哈希表来存储这些信息。C++11中的哈希表称为std::unordered_map。然后，您的查找操作可以在O(1)摊销时间内完成。有了这个映射，您的correct_login()函数就变得不必要了。

下面是重构的代码：

class bankAcc
{
public:
    bankAcc() = default;
    bankAcc(const string &fn, const string &ln)
    {
        if (fn == "" || ln == "")
            throw std::invalid_argument("Error! Can't have empty names");
        firstName = fn;
        lastName = ln;
    }
    bool createUser(const string &user_name, const string &pass_word)
    {
        if (user_name == "" || pass_word == "")
            return false;
        /* Maybe add more validation code here */
        // ....

        uname_to_pass.insert(user_name, pass_word);
    }
    bool login(const string & user_name, const string &pass) const
    {
        return uname_to_pass[user_name] == pass;
    }

private:
    static unordered_map<string, string> uname_to_pass;
    string firstName, lastName;
    string username, password;
};