A adição de um tipo de retorno a um método de atualização viola o “Princípio da Responsabilidade Única”?

Como acontece com qualquer regra, acho que o importante aqui é considerar o propósito da regra, o espírito, e não ficar atolado em analisar exatamente como a regra foi redigida em algum livro-texto e como aplicar isso a esse caso. Nós não precisamos abordar isso como advogados. O objetivo das regras é nos ajudar a escrever programas melhores. Não é como se o propósito de escrever programas fosse manter as regras.

O propósito da regra de responsabilidade única é tornar os programas mais fáceis de entender e manter, fazendo com que cada função faça uma coisa coerente e independente.

Por exemplo, uma vez eu escrevi uma função que chamei algo como "checkOrderStatus", que determinava se um pedido estava pendente, enviado, devolvido, o que quer que fosse, e retornava um código indicando qual. Em seguida, outro programador apareceu e modificou essa função para atualizar também a quantidade em mãos quando o pedido foi enviado. Isso violou severamente o princípio da responsabilidade única. Outro programador que lê esse código mais tarde verá o nome da função, verá como o valor de retorno foi usado e poderá nunca suspeitar que fez uma atualização do banco de dados. Alguém que precisava obter o status do pedido sem atualizar a quantidade disponível estaria em uma situação difícil: ele deveria escrever uma nova função que duplica a parte do status do pedido? Adicionar um sinalizador para informar se deve fazer a atualização do banco de dados? Etc. (Claro que a resposta certa seria quebrar a função em dois, mas isso pode não ser prático por muitas razões).

Por outro lado, eu não diria o que constitui "duas coisas". Recentemente, escrevi uma função que envia informações do cliente do nosso sistema para o sistema do nosso cliente. Essa função faz alguma reformatação dos dados para atender aos seus requisitos. Por exemplo, temos alguns campos que podem ser nulos em nosso banco de dados, mas eles não permitem nulos, então temos que preencher algum texto fictício, "não especificado" ou eu esqueço as palavras exatas. Indiscutivelmente esta função está fazendo duas coisas: reformatar os dados e enviá-los. Mas eu deliberadamente coloquei isso em uma única função, em vez de "reformatar" e "enviar", porque eu não quero nunca, jamais, enviar sem reformatar. Eu não quero que alguém escreva uma nova chamada e não perceba que ele tem que chamar reformatar e depois enviar.

No seu caso, atualizar o banco de dados e retornar uma imagem do registro escrito parece duas coisas que podem ir juntas logicamente e inevitavelmente. Não conheço os detalhes da sua inscrição, por isso não posso dizer definitivamente se é uma boa ideia ou não, mas parece plausível.

Se você estiver criando um objeto na memória que armazena todos os dados do registro, fazendo as chamadas do banco de dados para escrever isso e retornando o objeto, isso faz muito sentido. Você tem o objeto em suas mãos. Por que não apenas devolvê-lo? Se você não retornou o objeto, como o chamador conseguiria? Ele teria que ler o banco de dados para obter o objeto que você acabou de escrever? Isso parece bastante ineficiente. Como ele encontraria o registro? Você conhece a chave primária? Se alguém declarar que é "legal" para a função write retornar a chave primária para que você possa reler o registro, por que não apenas retornar o registro inteiro para que você não precise fazer isso? Qual a diferença?

Por outro lado, se criar o objeto é um trabalho bastante distinto da gravação do registro do banco de dados, e um chamador pode querer fazer a gravação mas não criar o objeto, então isso pode ser um desperdício. Se um chamador quiser o objeto, mas não escrever, então você deve fornecer outra maneira de obter o objeto, o que poderia significar escrever código redundante.

Mas acho que o cenário 1 é mais provável, então, eu diria, provavelmente sem problemas.

O conceito do SRP é parar os módulos fazendo duas coisas diferentes ao fazê-los individualmente para uma melhor manutenção e menos espaguete no tempo. Como o SRP diz "um motivo para mudar".

No seu caso, se você tivesse uma rotina que "atualiza e retorna o objeto atualizado", você ainda está alterando o objeto uma vez - dando a ele um motivo para mudar. Se você devolver o objeto de volta não está nem aqui nem lá, você ainda está operando nesse único objeto. O código é responsável por apenas uma coisa.

O SRP não é realmente sobre tentar reduzir todas as operações para uma única chamada, mas para reduzir o que você está operando e como você está operando nele. Portanto, uma única atualização que retorna o objeto atualizado está bem.

Como sempre, esta é uma questão de grau. O SRP deve impedir que você escreva um método que recupere um registro de um banco de dados externo, realize uma transformação rápida de Fourier e atualize um registro de estatísticas globais com o resultado. Eu acho que quase todo mundo concordaria que essas coisas deveriam ser feitas por métodos diferentes. Postular uma responsabilidade única para cada método é simplesmente a maneira mais econômica e memorável de fazer isso.

No outro extremo do espectro, há métodos que geram informações sobre o estado de um objeto. O típico isActive tem essa informação como única responsabilidade. Provavelmente todos concordam que está tudo bem.

Agora, alguns estendem o princípio até o momento de considerar a devolução de uma bandeira de sucesso a uma responsabilidade diferente de executar a ação cujo sucesso está sendo relatado. Sob uma interpretação extremamente estrita, isso é verdade, mas como a alternativa seria ter que chamar um segundo método para obter o status de sucesso, o que complica o chamador, muitos programadores estão perfeitamente bem com códigos de sucesso retornados de um método com efeitos colaterais. / p>

A devolução do novo objeto está um passo adiante no caminho para múltiplas responsabilidades. Exigir que o chamador faça uma segunda chamada para um objeto inteiro é um pouco mais razoável do que exigir uma segunda chamada apenas para ver se a primeira foi bem-sucedida ou não. Ainda assim, muitos programadores considerariam retornar o resultado de uma atualização perfeitamente bem. Embora isso possa ser interpretado como duas responsabilidades ligeiramente diferentes, certamente não é um dos abusos flagrantes que inspirou o princípio.

Isso viola o Princípio da Responsabilidade Única?

Não necessariamente. No mínimo, isso viola o princípio da separação consulta de comandos .

A responsabilidade é

update the employee data

com o entendimento implícito de que essa responsabilidade inclui o status da operação; por exemplo, se a operação falhar, uma exceção é lançada, se for bem-sucedida, o funcionário da atualização será retornado, etc.

Novamente, tudo isso é questão de grau e julgamento subjetivo.

Então, o que acontece com a separação de comandos e consultas?

Bem, esse princípio existe, mas retornar o resultado de uma atualização é de fato muito comum.

(1) Set#add(E) adiciona o elemento e retorna sua inclusão anterior no conjunto.

if (visited.add(nodeToVisit)) {
    // perform operation once
}

Isso é mais eficiente que a alternativa do CQS, que pode ter que realizar duas pesquisas.

if (!visited.contains(nodeToVisit)) {
    visited.add(nodeToVisit);
    // perform operation once
}

(2) Compare-e-swap , fetch-and-add e test -e-set são primitivos comuns que permitem a existência de programação simultânea. Esses padrões aparecem com frequência, desde instruções de CPU de baixo nível até coleções simultâneas de alto nível.

A única responsabilidade é sobre uma classe que não precisa mudar por mais de um motivo.

Como exemplo, um funcionário possui uma lista de números de telefone. Quando a maneira como você lida com números de telefone mudar (você pode adicionar códigos de chamada do país) que não deve mudar a classe de funcionários em tudo.

Eu não precisaria que a classe do funcionário soubesse como ela se salva no banco de dados, porque ela mudaria para alterações nos funcionários e mudanças na forma como os dados são armazenados.

Semelhante não deve haver um método CalculateSalary na classe employee. Uma propriedade salarial é boa, mas o cálculo do imposto etc. deve ser feito em outro lugar.

Mas o método Update retorna o que ele acabou de atualizar.

o caso específico:

Employee Update(Employee, name, password, etc) (actually using a Builder since I have a lot of parameters).

parece que o método Update usa um Employee como primeiro parâmetro para identificar (?) o funcionário existente e um conjunto de parâmetros para alterar esse funcionário.

Eu faço acho que isso poderia ser feito mais limpo. Eu vejo dois casos:

(a) Employee na verdade contém um banco de dados / ID exclusivo pelo qual ele sempre pode ser identificado no banco de dados. (Ou seja, você não precisa de todos os valores de registro definidos para encontrá-lo no banco de dados.

Nesse caso, eu preferiria um método void Update(Employee obj) , que apenas localiza o registro existente por ID e atualiza os campos do objeto passado. Ou talvez um void Update(ID, EmployeeBuilder values)

Uma variação disso que achei útil é ter apenas um método void Put(Employee obj) que insere ou atualiza, dependendo se o registro (por ID) existe.

(b) O registro completo existente é necessário para a pesquisa do banco de dados, nesse caso, ainda faz mais sentido ter: void Update(Employee existing, Employee newData) .

Até onde eu posso ver aqui, eu realmente diria que a responsabilidade de construir um novo objeto (ou valores de subobjetos) para armazenar e realmente armazená-lo é ortogonal, então eu os separaria.

Os requisitos concorrentes mencionados em outras respostas (atômica set-and-retrieve / compare-swap, etc.) não foram um problema no código do DB no qual trabalhei até agora. Ao falar com um banco de dados, acho que isso deve ser tratado no nível da transação normalmente, não no nível de instrução individual. (Isso não quer dizer que pode não haver um design onde "atomic" Employee[existing data] Update(ID, Employee newData) não possa fazer sentido, mas com acessos ao DB não é algo que eu normalmente vejo.)

Até agora todo mundo aqui está falando sobre classes. Mas pense nisso de uma perspectiva de interface.

Se um método de interface declarar um tipo de retorno e / ou uma promessa implícita sobre o valor de retorno, toda implementação precisará retornar o objeto atualizado.

Então você pode perguntar:

• Você consegue pensar em possíveis implementações que não querem incomodar o retorno de um novo objeto?
• Você pode pensar em componentes que dependem da interface (com uma instância injetada), que não precisam do funcionário atualizado como um valor de retorno?

Pense também em objetos simulados para testes de unidade. Obviamente, será mais fácil zombar sem o valor de retorno. E os componentes dependentes são mais fáceis de testar, se suas dependências injetadas tiverem interfaces mais simples.

Com base nessas considerações, você pode tomar uma decisão.

E se você se arrepender mais tarde, ainda poderá introduzir uma segunda interface, com adaptadores entre os dois. É claro que essas interfaces adicionais aumentam a complexidade de composição, então é tudo uma desvantagem.

Sua abordagem está bem. A imutabilidade é uma afirmação strong. A única coisa que gostaria de perguntar é: existe algum outro lugar onde você constrói o objeto. Se o seu objeto não fosse imutável, você teria que responder a perguntas adicionais porque "Estado" é apresentado. E a mudança de estado de um objeto pode ocorrer por diferentes razões. Então você deve conhecer seus casos e eles não devem ser redundantes ou divididos.