个人财务管理工具(OOP版本)

Question

CodeReview社区！我希望你能复习一下我的基本OOP程序。去年，我做了一次严格的过程处理，现在我已经整合了OOP技术。忽略注释掉的代码段。匈牙利符号是我大学课程的一项要求，所以请不要批评我使用过它。我想知道什么是我做得好，哪些是我没有做的，以及在使我的代码更好的过程中的任何建议。在此之前，非常感谢您。下面的场景是我所建立的程序的基础：

一家信誉良好的银行要求您创建一个个人财务管理程序。该程序将需要能够获取用户的月工资，他们的每月账单，以及任何可能发生的每周账单。然后，我们将对这些数据进行细分，找出他们的工资中有多少是留给储蓄的。用户应该能够输入任何金额的每月帐单和每周帐单。用户还可以选择在计算中添加家中的其他用户。一旦将所有相关信息都包括在内，就应该根据每周、每月和每年的成本对账单进行概述。输入用户名(必须大于1)月工资(必须大于1)用户必须支付的不同账单(每人)输出用户名周刊、月工资、年工资总额，每月、年度账单支出总额

// Personal Finance Tool.cpp : This file contains the 'main' function. Program execution begins and ends there.
//

#include <iostream>
#include <vector>
#include <string>
#include <memory>
#include "Utils.h"
class Bill 
{
private:
    std::string m_sBillName;
    double m_dMonthlyBill;
public:
    Bill(std::string sBillName, double dMonthlyBill)
    :m_sBillName(sBillName),m_dMonthlyBill(dMonthlyBill) {}
    double GetMonthlyBill() {
        return m_dMonthlyBill;
    }
};

class User 
{
private:
    std::string m_sName;
    double m_dMonthlyWage;
    std::vector<Bill>objBillsToPay;
public:
    User(std::string sName, double dMonthlyWage)
    :m_sName(sName), m_dMonthlyWage(dMonthlyWage) {}
    
    void AddBill(std::string sBillName, double dMonthlyBill) 
    {
        objBillsToPay.push_back({ sBillName, dMonthlyBill });
    }

    double WeeklyWage() {
        return m_dMonthlyWage / 4;
    }

    double YearlyWage() {
        return m_dMonthlyWage * 12;
    }

    double TotalSpentOnBills()
    {
        double dTotal = 0;
        for (Bill &objBill : objBillsToPay) 
        {
            dTotal += objBill.GetMonthlyBill();
        }
        return dTotal;
    }

    double LeftToSaveMonthly()
    {
        return (YearlyWage() / 12) - (TotalSpentOnBills() / 12);
    }

    double TotalLeftToSaveYearly() 
    {
        return YearlyWage() - TotalSpentOnBills();
    }

    double OverSaved() 
    {
        return TotalLeftToSaveYearly() * 0.10;
    }

    double UnderSaved() {
        return TotalLeftToSaveYearly() - (TotalLeftToSaveYearly() * 0.10);
    }
};

void DisplayResults(std::vector<User>& objUsers) {
    for (User& objUser : objUsers) {
        std::cout << "***" << std::endl;
        std::cout << "Weekly wage: " << objUser.WeeklyWage() << std::endl;
        std::cout << "Yearly wage: " << objUser.YearlyWage() << std::endl;
        std::cout << "10% over total that can be saved: " << objUser.OverSaved() << std::endl;
        std::cout << "10% under total that can be saved: " << objUser.UnderSaved() << std::endl;
        std::cout << "Total left to save yearly: " << objUser.TotalLeftToSaveYearly() << std::endl;
        std::cout << "Total spent on bills: " << objUser.TotalSpentOnBills() << std::endl;  
    }
    std::cout << std::endl;
}

void TestData(std::vector<User>& objUsers)
{
    objUsers.push_back({ "Jack Kimmins", 764});
    objUsers.at(0).AddBill("Water Bill", 65);
    objUsers.push_back({ "George Bradley", 332});
    objUsers.push_back({ "Jason Hill", 343 });
    objUsers.push_back({ "Sean Shearing", 374 });
}

int main()
{
    std::vector<User>objUsers;
    TestData(objUsers);
    //CreateUser(objUsers); Will need to be wrapped in a while loop if uncommented
    DisplayResults(objUsers);
    system("pause"); //Ignore this, I know using system isn't good, just to stop it
}

Answer 1

欢迎来到CR社区！

避免为非内置类型的

复制。

作为一条很好的经验法则，传递不需要通过常量引用修改的大型对象。

这将确保您不会创建额外的副本。但是，您不需要对内置类型如int、float、char、double.

如果您有一个变量x。通过常量引用传递它意味着const type& x。这里所做的基本上是传递内存地址，该地址被复制到x中。这样，您就不会传递整个string类。而是内存中的一个地址，它指向一个现有的字符串类。

如果你想要什么东西送到你的房子，你会给公司你的地址还是你的房子？

为什么是const？这确保您不会意外地修改x的值。因为如果你这样做了，这个值就会在任何地方发生变化。

到处都是？我是说你以前调用这个函数的值。

void fun(int& x)
{
    x = 5;
}

int x = 10;
fun(); // x is now 5!

使用const会告诉编译器，“如果你看到我修改它，说点什么，这样我就可以避免噩梦了”。

更喜欢使用'\n'结束std::endl

std::endl每次都给std::flush打电话。这使得它比打印'\n'效率低。他们俩都会在这里完成这份工作，除了其中一个会更快。

例如

std::cout << "***" << std::endl;

这将调用<<操作符两次+调用std::flush。可以通过以下方法实现相同的换行符

std::cout << "***\n";

这样会更有效率

保持一致的格式

您的代码格式不一致。您的IDE应该能够轻松地将代码格式化为不同的样式，或者即使是像这样的网站也可以完成这项工作。

将代码重新格式化为不同的文件

随着代码的增长，您将意识到不可能在main.cpp中拥有所有的代码。这样做会降低代码的可读性。稍后，您会发现维护代码也会更加困难。如果你只有Bill.h和User.h呢！这样，每次遇到问题时，都可以轻松地导航到这些文件。此外，代码看起来要干净得多。

重新格式化为单独的文件

Bill.h

#include <string>

class Bill 
{
    private:
        std::string m_sBillName;
        double m_dMonthlyBill;
        
    public:
        Bill(std::string,double);
        double GetMonthlyBill();
};

Bill.cpp

#include "Bill.h"


Bill::Bill(std::string sBillName, double dMonthlyBill)
    :m_sBillName(sBillName),m_dMonthlyBill(dMonthlyBill) 
    
    {}

double Bill::GetMonthlyBill() {
    return m_dMonthlyBill;
}

User.h

#include <string>
#include <vector>
#include "Bill.h"

class User 
{
    private:
        std::string m_sName;
        double m_dMonthlyWage;
        std::vector <Bill> objBillsToPay;
        
    public:
        User(std::string,double);
        void AddBill(std::string,double);
        double WeeklyWage();
        double YearlyWage();
        double TotalSpentOnBills();
        double LeftToSaveMonthly();
        double TotalLeftToSaveYearly();
        double OverSaved();
        double UnderSaved();
    
};

User.cpp

#include "User.h"



User::User(std::string sName, double dMonthlyWage)
        :m_sName(sName), m_dMonthlyWage(dMonthlyWage) 
    {}
    
void User::AddBill(std::string sBillName, double dMonthlyBill) 
{
    objBillsToPay.push_back({ sBillName, dMonthlyBill });
}

double User::WeeklyWage() {
    return m_dMonthlyWage / 4;
}

double User::YearlyWage() {
    return m_dMonthlyWage * 12;
}

double User::TotalSpentOnBills()
{
    double dTotal = 0;
    for (Bill &objBill : objBillsToPay) 
    {
        dTotal += objBill.GetMonthlyBill();
    }
    return dTotal;
}

double User::LeftToSaveMonthly()
{
    return (YearlyWage() / 12) - (TotalSpentOnBills() / 12);
}

double User::TotalLeftToSaveYearly() 
{
    return YearlyWage() - TotalSpentOnBills();
}

double User::OverSaved() 
{
    return TotalLeftToSaveYearly() * 0.10;
}

double User::UnderSaved() {
    return TotalLeftToSaveYearly() - (TotalLeftToSaveYearly() * 0.10);
}

• std::string m_sBillName;在Bill做什么？您已经声明了它是private，但是您没有在任何成员函数中使用它，而且由于它是私有的，所以它也不能在外部访问。

Answer 2

避免匈牙利符号

您应该使用避免使用匈牙利符号，特别是如果要用obj作为所有非原始类型的前缀。在这一点上它真的变得毫无用处了。您也无法区分通过值传递的对象和通过引用传递的对象。

但是，使用m_前缀标记成员变量是正确的，因此保留它没有问题。

为什么Bill有名字？

你给Bills起名字，但你永远读不回这个名字。在那一点上，这是无用的信息。如果您不存储名称，那么只剩下变量dMonthlyBill，它只是一个double。如果仅此而已，我要说的是，这门课没有任何用处，应该删除。相反，您可以在class User中编写：

std::vector<double> monthly_bills;

但是，class User也是如此。没有办法把名字或账单清单从里面拿出来。它唯一真正需要储存的是月工资和每月账单的总和：

class User
{
    double m_monthly_wage;
    double m_monthly_bills = 0;

public:
    User(double monthly_wage): m_monthly_wage(monthly_wage) {}

    void AddBill(double monthly_bill) {
        m_monthly_bills += monthly_bill;
    }

    ...
}

在时间段之间要小心转换

你不能盲目地把月薪和周薪除以四。毕竟，一年有52.1775周，而不是48周！

此外，LeftToSaveMonthly()中的计算是错误的：TotalSpentOnBills()已经是每个月了，所以您不应该将其除以12，并且不需要调用YearlyWage()，然后再将答案除以12。你只需写：

double LeftToSaveMonthly()
{
    return m_dMonthlyWage - TotalSpentOnBills();
}

避免使用system()

不要用system()，除非你真的需要它，它是非常低效率和不可移植的。在这种情况下，您可以简单地写：

std::cin.get();

这可能需要用户按Enter而不是任何键(如果输入是行缓冲的)，但这应该是好的。

您已经在代码中的注释中写了一些您知道它不好的注释，但是为什么这样做呢？