Pergunta

Say você tem estes dois métodos:

Number 1:

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

Número 2:

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

Qual dos dois métodos é o melhor? Eu sei que o primeiro é mais correto OO-sábio, mas eu sinto o segundo é mais legível, e você não tem que verificar se o objeto é válido como os parâmetros se certificar disso. Eu só não gosto de ter que validar os objetos em qualquer lugar eu passá-los como parâmetros. Existem outras abordagens?

EDIT: Thx para todas as respostas. Para esclarecer, validando no construtor e um método IsValid é, claro, uma boa abordagem, mas no meu código do estado válido da pessoa é muitas vezes dependente do contexto e pode variar de método para método. Isso pode, naturalmente, ser um sinal de design ruim.

O código é apenas um exemplo para descrever o problema.

Foi útil?

Solução

O primeiro não deveria ter que validar person.Name e person.BirthDate - eles devem ser validados automaticamente pelo construtor Person. Em outras palavras, se você estiver aprovou uma pessoa, você deve saber que ele é válido.

Por outro lado, você teria que verificar que person não é uma referência nula.

Às vezes vale a pena ter métodos de conveniência como a segunda versão para evitar ter que chamar explicitamente o construtor tão frequentemente embora. Deve geralmente apenas chamar o construtor Person e, em seguida, delegar o trabalho para a primeira forma.

Outras dicas

O primeiro tem a vantagem de permitir que você altere a definição Pessoa sem quebrar o código existente, é apenas necessário recompilação. Você pode pensar que a segunda é mais legível, mas o primeiro é mais sustentável, a sua escolha.

Outra opção seria:

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

Assumindo que essa pessoa não validar a si mesmo quando ela é construída. Que em alguns casos é viável se o objeto pode ser um estado inválido enquanto é transitória.

Eu prefiro a primeira (passando um objeto), porque ele reduz o acoplamento da API para o objeto. Se você alterar o objeto Person, por exemplo, adicionar uma nova propriedade, como Nickname que não era necessário anteriormente, em seguida, no primeiro caso, você não precisa mudar a API pública, enquanto no segundo você precisa se quer mudar o método ou adicionar uma nova sobrecarga.

Eu concordo que depende inteiramente do contexto, não há regras absolutas para isso. Na minha opinião, seria um absurdo ter métodos como estes:

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

Mas neste caso eu prefiro o primeiro, porque, como disse Greg Beech, o método não (tem que) sabe nada sobre o objeto de domínio.

A propósito, considere a sobrecarga (DRY - Do not Repeat Yourself):

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);
}

Definitivamente melhor para passar um objeto pessoa ao redor, em vez de um monte de tipos primitivos como parâmetros. Comparar os dois métodos a seguir


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);
}

Os dois métodos parecem quase idênticas, mas o segundo não é seguro. Um int pode vir de qualquer lugar, então você está ferrado se você escrever o seguinte:

private void CloseTransaction(Transaction tran)
{
    BankAccounts.Withdrawal(tran.Account.RoutingNumber, tran.Amount);
        // logic error: meant to pass Account.AccountNumber instead of Account.RoutingNumber
}

Este é o pior tipo de erro porque ele não vai jogar um erro de compilação ou uma exceção de tempo de execução. Você pode pegar esse erro em seus testes automatizados, se você escrever bem o suficiente, mas este bug é fácil perder e pode ser capaz de esconder por meses sem ser descoberto.

Eu trabalhei uma empresa que escreveu software de banco, e nós realmente funciona através de um bug deste tipo na produção. Isso só ocorreu durante um tipo específico de transferência de abóbada, e só foi descoberta quando um de nossos bancos notado sua GL equilibra fora por alguns $ 100 cada vez que correu o seu processo de fim de mês. O banco suspeita de roubo do empregado durante meses, mas foi apenas através de revisão de código cuidadoso que alguém traçou o problema a um bug no nosso software.

Eu maior parte do tempo ir para o primeiro.

O segundo iria quebrar a assinatura para cada mudança na Person

Depende do contexto.

Se todos os seus métodos de chamada lidar com objetos Person, em seguida, o primeiro é a solução correta.

No entanto, se alguns dos seus métodos de chamada lidar com nome e datas de nascimento, e alguns lidar com objetos Pessoa, em seguida, o segundo exemplo é a solução correta.

Gostaria apenas de criar sobrecargas para ambos. Especialmente considerando que eles podem ser criados automaticamente.

Estou assumindo que os métodos não são parte do tipo Person. Com isso em mente eu sinto que ambos têm um pouco demais conhecimento sobre outro tipo (Person) imo. A primeira versão não deveria ter que validar a criação de uma instância de pessoa válida. Se esta é a responsabilidade do chamador, então cada chamador deve fazer isso. Isso é frágil.

A segunda versão tem uma forte dependência para outro tipo devido ao fato de que ele cria uma instância desse tipo.

Eu definitivamente prefiro a primeira versão, mas eu gostaria de mover a parte de validação de que parte do código.

Eu acho que Jon praticamente acertou em cheio. Person deve ser responsabilidade de assegurar uma Pessoa válido é criado, seu construtor.

Sobre o resonsibility que cria ou não cria o objeto Person (se o método AddPerson ou seu chamador), leia

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

Trata-se de questões de responsabilidade em OOP. No seu caso específico, se o AddPerson envolvendo a chamada para uma interface DB, eu não estou muito certo sobre isso. Depende o que essa pessoa objeto é utilizado para fora desse contexto. Se é unicamente com a finalidade de conter os dados a serem adicionados ao banco de dados, criando-lo no método AddPerson possivelmente é uma boa idéia, porque separa o usuário de sua classe de ter que saber sobre a classe Pessoa. .

Melhor encapsulamento se você usar objetos; isso é o que eles são para. Acho que as pessoas entrar em apuros quando eles usam primitivos demais.

Não deve ser possível criar uma pessoa inválida. O construtor deve verificar se há parâmetros válidos e jogar IllegalArgumentException ou não uma declaração se eles são inválidos. Isto é o que de programação por contrato é tudo.

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top