ISBN类-有效ISBN 13的检查

Question

我现在正试图掌握多文件编程，而不是一个大的main.cpp。

我写的课程是关于ISBN 13的。

您能指出如何改进我的代码吗？

还有一个好的指南，如何写包含警卫？我的线人在这方面不一样。

Wandbox上的可运行代码

ISBN.hpp

#include <ostream>

class ISBN
{
/* examples 978-345-314-697-6
978-3-12-732320-7
*/
private:
    //the numbers between the minus signs
    unsigned int isbn_field1;
    unsigned int isbn_field2;
    unsigned int isbn_field3;
    unsigned int isbn_field4;
    unsigned int isbn_field5;
public:
    //constructors
    ISBN()
        : ISBN{978,3,12,732320,7} // a valid ISBN as default
    {}
    ISBN(const unsigned int& a, const unsigned int& b, const unsigned int& c, const unsigned int& d, const unsigned int& e);

    //output operator overloading
    friend std::ostream& operator<<(std::ostream& os, ISBN const& i);
};//ISBN

ISBN.cpp

#include "ISBN.hpp"
#include <iostream>
#include <string>

bool is_valid_isbn(const std::string& digits)
{
    unsigned int weighted_sum=0;
    std::string temp;
    for (unsigned int i=0; i<(digits.length());i++)
    {
        temp=digits[i];
        if ((i%2)==0)
        {
            weighted_sum+=std::stoi(temp);
        }
        else
        {
            weighted_sum+=std::stoi(temp)*3;
        }
    }
    if ((weighted_sum%10)!=0)
    {
        return false;
    }
    return true;
}

ISBN::ISBN(const unsigned int& a, const unsigned int& b, const unsigned int& c, const unsigned int& d, const unsigned int& e)
{
    const std::string digit_string=std::to_string(a)
    +std::to_string(b)
    +std::to_string(c)
    +std::to_string(d)
    +std::to_string(e);
    std::string temp;
    //check if last field is exactly one digit
    temp=std::to_string(e);
    if(temp.length()!=1)
    {
        throw "last unit is not a single digit\n";
    }
    //check if first field is 978 or 979
    if(a!=978 && a!=979)
    {
        throw "first unit is neither 978 nor 979\n";
    }
    //check number of digits=13
    if(digit_string.length()!=13)
    {
        throw "number of digits is not exactly 13\n";
    }
    if (!is_valid_isbn(digit_string))
    {
        throw "not a valid ISBN\n";
    }

    isbn_field1=a;
    isbn_field2=b;
    isbn_field3=c;
    isbn_field4=d;
    isbn_field5=e;
}

std::ostream& operator<<(std::ostream& os, ISBN const& i)
{
    return os << i.isbn_field1 << "-"
    << i.isbn_field2 << "-"
    << i.isbn_field3 << "-"
    << i.isbn_field4 << "-"
    << i.isbn_field5;
}

main.cpp

#include "ISBN.hpp"

#include <iostream>


int main()
{
    try
    {
        ISBN test;
        std::cout << test;
    }
    catch(char const* e)
    {
        std::cout << "Error:" << e;
    }
    catch(...)
    {
        std::cout << "test error";
    }
    return 0;
}

Answer 1

ISBN类中的注释应该在类声明之外，并且应该包含一些供读者使用的单词，例如扩展的缩写和维基百科页面，以便更容易地获得有关ISBN主题的其他信息。

将ISBN存储为5个数字是错误的。如果这些数字的长度不等于13，则无法确定在哪里添加零。维基百科文章包含一个示例ISBN-10数字表，其中一些数字有前导数字。

变量名isbn_field1是坏的。isbn这个词是不必要的，因为ISBN类已经给出了这个信息。因此，field1优于isbn_field1。但是，field1是什么，它意味着什么？维基百科的文章为这些字段提供了很好的名称，您的代码也应该这样做。

数字之间的字符不是“减号”(如你所说)，而是连字符。

无参数构造函数是错误的。如果不指定ISBN对象的所有部分，就不应该构造ISBN对象。对于ISBN来说，拥有一个单一的“最佳默认值”是不可能的。你肯定不想把你所有的编程错误都转化成解析几何的书。

在带有const unsigned int&参数的构造函数中，const&是不必要的。整数值通常是直接传递的，而不是引用它们，因为它们是一个机器字长，而且计算机可以快速处理它们。在这种情况下，引用会使代码运行得更慢。对于较大的类型，如std::vector<std::vector<ISBN>>，使用引用而不是简单的值是有意义的。

在is_valid_isbn函数中，您应该在i=0中的令牌周围添加空格。大多数C++程序员习惯于将其理解为i = 0。

在同一行中，digits.length()周围的括号是不必要的。

与调用std::stoi不同，您还可以编写digits[i] - '0'，这将为您提供一个数字字符的数值。这提醒了我，有些字符可能根本不是数字。您的代码应该跳过它们，然后验证字符串总共包含13位数字。

在ISBN::ISBN构造函数中，与检查最后一个字段是否长度为1不同，您只需检查是否为e < 10。这既简单又快捷。

在验证多个值时，应按读取顺序验证它们。从第一个字段开始，最后验证最后一个字段。

您没有通过调用ISBN{0, 0, 0, 0, 0}来测试代码。

当您从代码中throw某些内容时，它应该是C++标准库(如std::invalid_argument )中的一个适当的例外。

您可以简单地编写std::string temp; temp = std::to_string(e); temp.length() != 1，而不是冗长的std::to_string(e).length() != 1。这样，您就不必考虑更好的变量名而不是temp。名称temp总是一个糟糕的变量名，因为它没有足够的讲述一个故事。(嗯，除非你用它作为温度的缩写，但这里不是这样的。)

异常字符串不应包含换行符。在日志文件中打印异常时，应添加该字符。如果您想将异常保存到其他地方，换行符可能是错误的。

main程序是非常无用的。无法输入ISBN并对其进行验证。您应该定义以下函数以获得一些自动测试：

void assert_valid_isbn(const std::string &isbn)
{
    /* TODO */
}

void assert_invalid_isbn(const std::string &isbn, const std::string &expected_error)
{
    /* TODO */
}