2

我正在实现一个类来表示旧文件。这些文件是二进制文件，需要大量解析。可能会有数千行代码来解析文件。我在类中有两个构造函数：

class CrmxFile {

public:
    CrmxFile(std::wstring filename);
    CrmxFile(std::ifstream& file);

...
}

由于没有文件的内容，对象就没有意义，所有的解析都需要在构造对象时完成。我打算实现一个大型构造函数（带有流参数的构造函数），但我的一位同事认为拥有非常大的构造函数不是一个好习惯。相反，我应该创建一个私有方法来进行解析，并在检查流是否可读之后从构造函数调用此方法，并且可能读取文件头以验证流是否包含正确的数据。

是否有任何指导方针/规则/约定/等。管理这种情况？本质上（在伪代码中），这两种方法中的哪一种更好，

长构造函数：

CrmxFile::CrmxFile(std::ifstream& file) {
    if (file is not readable) {
        throw new CmrxException(read_error);
    }

    CmrxHeader header(file);
    if(!header.isValid()) {
        throw new CmrxException(invalid_file);
    }

    //now do all the reading/parsing of the file
    //constructor may end up being a thousand lines of code
    ...
}

或简短的构造函数和辅助方法：

class CrmxFile {

public:
    CrmxFile(std::wstring filename);
    CrmxFile(std::ifstream& file);

private:
    ParseFile(std::ifstream& file);

...
}

CrmxFile::CrmxFile(std::ifstream& file) {
    if (file is not readable) {
        throw new CrmxException(read_error);
    }

    CrmxHeader header(file);
    if(!header.isValid()) {
        throw new CrmxException(invalid_file);
    }

    ParseFile(file);
}

void CrmxFile::ParseFile(std::ifstream& file) {
    //do all the reading/parsing of the file here
    ...
}
4

4 回答 4

5

帮手在这里似乎可行..

但是，我不会将其设为私有：而是将其拆分为多个驻留在详细信息或解析器命名空间中的尽可能小的函数，以便您可以测试每个方法。由于您无论如何都使用流，这很容易：将您的方法更改为接受 anistream而不是，ifstream然后测试的输入可以是纯字符串，然后您将其输入到您用作方法参数的 istringstream 中。相信我，你会想要测试这个，正如你现在所说的那样，它将有数百行，你不可能从第一次就做对。

实际的辅助方法不是简单地组合较小的方法，或者根据您拥有的数量，组合较小的方法，然后再组合其他方法。

于 2013-01-04T12:39:25.377 回答
0

如果您有多个构造函数，那么最好有一个私有方法来进行初始化。节省重复代码（从而有助于在进行任何更改时更易于维护并且更不容易出错）。

如果只有一个构造函数，就让它变得尽可能复杂。

于 2013-01-04T12:37:27.937 回答
0

我认为这并不重要，除非您想共享私有帮助方法以供其他用途或对象内的多个构造函数。

在构造函数内部解析的方法对我来说听起来有问题。这是构造和初始化角色的混合。您可能希望将解析公开为公共接口，并且构造函数只会将对象的状态设置为未初始化，以将对象标记为对客户端不可用。或者，您可能希望在此处尝试使用惰性评估应用单例模式：实现getInstance将调用私有的方法（parse如果尚未解析） - 从用户的角度来看，它将是透明的（不包括首次访问的开销）。

于 2013-01-04T12:44:47.823 回答
0

我会说“不要在构造函数中这样做，句号”。

构造函数是奇怪的野兽，构造过程中的失败只有抛出异常才有可能。并且损坏/无效的文件或解析错误（在我看来）太常见而无法抛出 - 另外，在构建过程中进行抛出和解析安全是具有挑战性的，而且人们经常会出错。

所以走一条更简单的路。

编写一个工厂函数，甚至是一个工厂对象，来生成你的对象。它维护解析状态，并具有解析对象的无数函数（或者，如果必须的话，一个大函数）。然后，一旦解析完成，对象就会返回，并且该对象没有一堆与从磁盘解析它相关联的代码（或状态）。

这意味着而不是

try {
  MyObject foo("file.blah");
  // ...
} catch (MyObject::ParseError err) {
  // ...
}

您改为执行以下操作：

MyObjectParser parser;
parser.Load("file.blah");
if (parser.Error()) {
  // ...
} else {
  MyObject foo = parser.Get();
  // ...
}

并且您的情况是，唯一MyObject存在的应该MyObject是已经加载的商品。

于 2013-01-04T13:45:52.487 回答