3

假设你有这两种方法：

1号：

void AddPerson(Person person)
{
  // Validate person
  if(person.Name != null && IsValidDate(person.BirthDate)
    DB.AddPersonToDatabase(person);
}

2号：

void AddPerson(string name, DateTime birthDate)
{
  Person p = new Person(name, birthDate);
  DB.AddPersonToDatabase(person);
}

这两种方法中哪一种是最好的？我知道第一个在 OO 方面更正确，但我觉得第二个更具可读性，您不必确保对象有效，因为参数可以确保这一点。我只是不喜欢在将对象作为参数传递的任何地方验证对象。还有其他方法吗？

编辑：谢谢所有的答案。澄清一下，在构造函数和 IsValid 方法中进行验证当然是一种好方法，但在我的代码中，人的有效状态通常取决于上下文，并且可能因方法而异。这当然可能是糟糕设计的标志。

该代码只是描述问题的示例。

4

12 回答 12

10

第一个不必验证 person.Name 和 person.BirthDate - 它们应该由Person构造函数自动验证。换句话说，如果你传递了一个 Person，你应该知道它是有效的。

另一方面，您必须检查这person不是空引用。

有时值得拥有像第二个版本这样的便利方法，以避免经常显式调用构造函数。它通常应该只调用Person构造函数，然后将工作委托给第一个表单。

于 2008-12-31T14:28:41.993 回答
2

第一个的优点是允许您在不破坏现有代码的情况下更改 Person 定义，只需要重新编译。您可能认为第二个更具可读性，但第一个更易于维护，这是您的选择。

于 2008-12-31T14:27:38.097 回答
2

另一种选择是：

void AddPerson(Person person)
{  // Validate person  
   if(person.IsValid)
   {
     DB.AddPersonToDatabase(person);
   }
}

假设 Person 在构造时不验证自己。如果对象在瞬态时可以是无效状态，则在某些情况下这是可行的。

于 2008-12-31T14:31:23.563 回答
1

我更喜欢前者（传递对象），因为它减少了 API 与对象的耦合。如果您更改Person对象，例如添加一个Nickname以前不需要的新属性，那么在第一种情况下您不需要更改公共 API，而在第二种情况下您需要更改方法或添加一个新的过载。

于 2008-12-31T14:28:42.977 回答
1

我同意这完全取决于上下文，对此没有绝对的规则。在我看来，有这样的方法是无稽之谈：

person.SetBirthDate(Person person)
person.ResetPassword(Person person)

但在这种情况下，我确实更喜欢前者，因为正如 Greg Beech 所说，该方法不需要（必须）了解有关域对象的任何信息。

顺便说一句，考虑重载（DRY - 不要重复自己）：

void AddPerson(Person person)
{
  if(person.Name != null && IsValidDate(person.BirthDate)
    DB.AddPersonToDatabase(person);
}

void AddPerson(string name, DateTime birthDate)
{
  Person p = new Person(name, birthDate);
  this.AddPerson(p);
}
于 2008-12-31T14:35:41.933 回答
1

绝对更好地传递一个 Person 对象，而不是一堆原始类型作为参数。比较以下两种方法


public static void Withdrawal(Account account, decimal amount)
{
    DB.UpdateBalance(account.AccountNumber, amount);
}

public static void Withdraw(int accountNumber, decimal amount)
{
    DB.UpdateBalance(accountNumber, amount);
}

这两种方法看起来几乎相同，但第二种方法不安全。一个 int 可以来自任何地方，所以如果你这样写，你会被搞砸的：

私人无效CloseTransaction（Transaction tran）
{
    BankAccounts.Withdrawal(tran.Account.RoutingNumber, tran.Amount);
        // 逻辑错误：本意是传递 Account.AccountNumber 而不是 Account.RoutingNumber
}

这是最糟糕的错误，因为它不会引发编译错误或运行时异常。如果你写得足够好，你可能会在你的自动化测试中发现这个错误，但是这个错误很容易被忽略，并且可能会隐藏几个月而不被发现。

我在一家编写银行软件的公司工作，我们确实在生产中遇到了这种类型的错误。它只发生在特定类型的金库转账期间，并且只有在我们的一家银行发现他们的 GL 余额每次运行月末流程时都会减少 100 美元时才被发现。银行怀疑员工盗窃数月之久，但只有通过仔细的代码审查，才有人将问题追溯到我们软件中的错误。

于 2008-12-31T15:50:29.363 回答
0

大多数时候我会选择第一个。

第二个会破坏 Person 的每次更改的签名

于 2008-12-31T14:29:00.950 回答
0

这取决于上下文。

如果您的所有调用方法都处理 Person 对象，那么第一个是正确的解决方案。

但是，如果您的一些调用方法处理姓名和出生日期，而一些处理 Person 对象，那么第二个示例是正确的解决方案。

于 2008-12-31T14:29:23.133 回答
0

我只会为两者创建重载。特别是考虑到它们可以自动创建。

于 2008-12-31T14:30:30.290 回答
0

我假设这些方法不是Person类型的一部分。Person考虑到这一点，我觉得他们俩都对另一种类型 ( ) imo了解得太多了。第一个版本不必验证有效人员实例的创建。如果这是调用者的责任，那么每个调用者都必须这样做。那是脆的。

第二个版本对另一个类型有很强的依赖性，因为它创建了这个类型的一个实例。

我肯定更喜欢第一个版本，但我会从那段代码中移走验证部分。

于 2008-12-31T14:37:02.117 回答
0

我认为乔恩几乎做到了。Person应该有责任确保在其构造函数中创建一个有效的 Person。

关于创建或不创建Person对象的责任（无论是 AddPerson 方法还是它的调用者），请阅读

http://en.wikipedia.org/wiki/GRASP_%28Object_Oriented_Design%29#Creator

这是关于 OOP 中的责任问题。在您的特定情况下，如果 AddPerson 将调用包装到数据库接口，我不太确定。这取决于该 Person 对象在该上下文之外用于什么。如果它只是为了包含要添加到数据库的数据，在 AddPerson 方法中创建它可能是一个好主意，因为它使您的类的用户不必了解 Person 类。.

于 2008-12-31T14:40:44.590 回答
0

如果使用对象，则更好的封装；这就是他们的目的。我认为人们在过度使用原语时会遇到麻烦。

应该不可能创建一个无效的人。构造函数应检查有效参数并抛出 IllegalArgumentException 或如果它们无效则使断言失败。这就是契约式编程的意义所在。

于 2008-12-31T15:05:50.720 回答