Você sempre deve passar os dados mínimos necessários para uma função em casos como este

Digamos que eu tenho uma função IsAdminque verifica se um usuário é um administrador. Digamos também que a verificação do administrador seja feita combinando a ID do usuário, o nome e a senha com algum tipo de regra (não importante).

Na minha cabeça, existem duas possíveis assinaturas de funções para isso:

public bool IsAdmin(User user);
public bool IsAdmin(int id, string name, string password);

Costumo escolher o segundo tipo de assinatura, pensando que:

• A assinatura da função fornece ao leitor muito mais informações
• A lógica contida na função não precisa saber sobre a Userclasse
• Geralmente resulta em um pouco menos de código dentro da função

No entanto, às vezes questiono essa abordagem e também percebo que em algum momento ela se tornaria pesada. Se, por exemplo, uma função mapeasse entre dez campos de objetos diferentes em um bool resultante, eu obviamente enviaria o objeto inteiro. Mas, além de um exemplo gritante como esse, não vejo uma razão para passar no objeto real.

Eu apreciaria quaisquer argumentos para qualquer estilo, bem como quaisquer observações gerais que você possa oferecer.

Como programa em estilos orientados a objetos e funcionais, a pergunta deve ser vista em relação a todo e qualquer idioma.

Eu pessoalmente prefiro o primeiro método de apenas IsAdmin(User user)

É muito mais fácil de usar e, se seus critérios para o IsAdmin forem alterados posteriormente (talvez com base em funções ou isActive), você não precisará reescrever a assinatura do método em todos os lugares.

Também é provavelmente mais seguro, pois você não anuncia quais propriedades determinam se um usuário é um administrador ou não, ou está passando a propriedade da senha em qualquer lugar. E syrion enfatiza que o que acontece quando o seu idnão corresponde ao name/ password?

O comprimento do código dentro de uma função não deve realmente importar, desde que o método faça seu trabalho, e eu prefiro ter um código de aplicativo mais curto e mais simples do que o código do método auxiliar.

A primeira assinatura é superior porque permite o encapsulamento da lógica relacionada ao usuário no Userpróprio objeto. Não é benéfico ter lógica para a construção da tupla "id, nome, senha" espalhada sobre sua base de código. Além disso, complica a isAdminfunção: o que acontece se alguém passa em um idque não corresponde ao name, por exemplo? E se você quiser verificar se um determinado usuário é um administrador de um contexto em que você não deve saber sua senha?

Além disso, como uma observação no seu terceiro ponto em favor do estilo com argumentos extras; pode resultar em "menos código dentro da função", mas para onde vai essa lógica? Não pode simplesmente desaparecer! Em vez disso, ele se espalha por todos os lugares onde a função é chamada. Para salvar cinco linhas de código em um único local, você pagou cinco linhas por uso da função!

A primeira, mas não (apenas) pelas razões que outros deram.

public bool IsAdmin(User user);

Esse é o tipo seguro. Especialmente porque User é um tipo que você definiu, há pouca ou nenhuma oportunidade de transpor ou alternar argumentos. Ele opera claramente em Usuários e não é uma função genérica que aceita int e duas strings. Essa é uma grande parte do ponto de usar uma linguagem de segurança de tipo como Java ou C # com a qual seu código se parece. Se você estiver perguntando sobre um idioma específico, poderá adicionar uma tag para esse idioma à sua pergunta.

public bool IsAdmin(int id, string name, string password);

Aqui, qualquer int e duas strings servirão. O que impede você de transpor o nome e a senha? O segundo é mais trabalho a ser usado e permite mais oportunidades de erro.

Presumivelmente, ambas as funções exigem que user.getId (), user.getName () e user.getPassword () sejam chamados, dentro da função (no primeiro caso) ou antes de chamar a função (no segundo), portanto, o A quantidade de acoplamento é a mesma. Na verdade, como essa função é válida apenas em Usuários, em Java eu ​​eliminaria todos os argumentos e tornaria isso um método de instância nos objetos Usuário:

user.isAdmin();

Como essa função já está fortemente acoplada aos Usuários, faz sentido torná-la parte do que é um usuário.

PS Tenho certeza de que este é apenas um exemplo, mas parece que você está armazenando uma senha. Em vez disso, você deve armazenar apenas um hash de senha criptograficamente segura. Durante o login, a senha fornecida deve ser hash da mesma maneira e comparada com o hash armazenado. O envio de senhas em texto sem formatação deve ser evitado. Se você violar essa regra e o isAdmin (int Str Str) tiver que registrar o nome de usuário, se você transpôs o nome e a senha no seu código, a senha poderá ser registrada, o que cria um problema de segurança (não escreva senhas nos logs ) e é outro argumento a favor de passar um objeto em vez de suas partes componentes (ou usar um método de classe).

Se o seu idioma de escolha não passa estruturas por valor, a (User user)variante parece melhor. E se algum dia você decidir descartar o nome de usuário e apenas usar o ID / senha para identificar o usuário?

Além disso, essa abordagem leva inevitavelmente a chamadas de método longas e exageradas, ou inconsistências. Passar 3 argumentos é bom? Que tal 5? Ou 6? Por que pensar nisso, se passar um objeto é barato em termos de recursos (ainda mais barato do que passar 3 argumentos, provavelmente)?

E eu (meio que) discordo que a segunda abordagem fornece ao leitor mais informações - intuitivamente, a chamada do método está perguntando "se o usuário tem direitos de administrador", e não "se a combinação de identificação / nome / senha fornecida tem direitos de administrador".

Portanto, a menos que a passagem do Userobjeto resulte na cópia de uma estrutura grande, a primeira abordagem é mais limpa e mais lógica para mim.

Eu provavelmente teria os dois. O primeiro como uma API externa, com alguma esperança de estabilidade no tempo. O segundo como uma implementação privada chamada pela API externa.

Se, em algum momento posterior, for necessário alterar a regra de verificação, escreveria apenas uma nova função privada com uma nova assinatura chamada pela externa.

Isso tem a vantagem da facilidade de alterar a implementação interna. Também é realmente comum que, em algum momento, você possa precisar das duas funções disponíveis ao mesmo tempo e chamar uma ou outra, dependendo de alguma mudança de contexto externa (por exemplo, você pode ter usuários antigos coexistentes e novos usuários com campos diferentes).

Em relação ao título da pergunta, de certa forma, nos dois casos, você está fornecendo as informações mínimas necessárias. Um usuário parece ser a menor estrutura de dados relacionados a aplicativos que contém as informações. Os três campos id, senha e nome parecem ser os menores dados de implementação realmente necessários, mas esses não são realmente o objeto no nível do aplicativo.

Em outras palavras, se você estivesse lidando com um banco de dados, um Usuário seria um registro, enquanto identificação, senha e login seriam campos nesse registro.

Há um ponto negativo para enviar classes (tipos de referência) para métodos, ou seja, Vulnerabilidade de atribuição em massa . Imagine que, em vez de IsAdmin(User user)método, eu tenho UpdateUser(User user)método.

Se a Userclasse tiver uma propriedade booleana chamada IsAdmine se eu não a verificar durante a execução do meu método, estou vulnerável à atribuição em massa. O GitHub foi atacado por essa mesma assinatura em 2012.

Eu iria com esta abordagem:

public bool IsAdmin(this IUser user) { /* ... */ }

public class User: IUser { /* ... */ }

public interface IUser
{
    string Username {get;}
    string Password {get;}
    IID Id {get;}
}

• O uso do tipo de interface como parâmetro para esta função permite passar objetos de Usuário, definir objetos de Usuário simulados para teste e oferece mais flexibilidade em relação ao uso e manutenção dessa função.

• Também adicionei a palavra this- chave para tornar a função um método de extensão de todas as classes IUser; Dessa forma, você pode escrever o código de maneira mais amigável ao OO e usar essa função nas consultas do Linq. Ainda mais simples seria definir essa função em IUser e User, mas suponho que haja algum motivo para você ter decidido colocar esse método fora dessa classe?

• Eu uso a interface personalizada em IIDvez de, intpois isso permite que você redefina as propriedades do seu ID; por exemplo, você deve precisar alterar a partir intde long, Guidou outra coisa. Provavelmente isso é um exagero, mas sempre é bom tentar flexibilizar as coisas para que você não fique preso às decisões iniciais.

• NB: O objeto IUser pode estar em um namespace e / ou assembly diferente da classe User, portanto, você tem a opção de manter o código do usuário completamente separado do código IsAdmin, pois eles compartilham apenas uma biblioteca referenciada. Exatamente o que você faz, há uma chamada para saber como você suspeita que esses itens sejam usados.

Isenções de responsabilidade:

Para minha resposta, vou me concentrar na questão do estilo e esquecer se um ID, login e senha simples são um bom conjunto de parâmetros a serem usados, do ponto de vista da segurança. Você deve conseguir extrapolar minha resposta para qualquer conjunto de dados básico que estiver usando para descobrir os privilégios do usuário ...

Eu também considero user.isAdmin()ser equivalente a IsAdmin(User user). Essa é uma escolha para você fazer.

Resposta:

Minha recomendação é apenas para o usuário:

public bool IsAdmin(User user);

Ou use ambos, ao longo das linhas de:

public bool IsAdmin(User user); // calls the private method below
private bool IsAdmin(int id, string name, string password);

Razões para o método público:

Ao escrever métodos públicos, geralmente é uma boa idéia pensar em como eles serão usados.

Nesse caso em particular, o motivo pelo qual você normalmente chama esse método é descobrir se um determinado usuário é um administrador. Esse é um método que você precisa. Você realmente não quer que cada chamador tenha que escolher os parâmetros certos para enviar (e arriscar erros devido à duplicação de código).

Razões para o método privado:

O método privado que recebe apenas o conjunto mínimo de parâmetros necessários pode ser uma coisa boa às vezes. Às vezes nem tanto.

Uma das coisas boas da divisão desses métodos é que você pode chamar a versão privada de vários métodos públicos da mesma classe. Por exemplo, se você deseja (por qualquer motivo) ter uma lógica ligeiramente diferente Localusere RemoteUser(talvez haja uma configuração para ativar / desativar administradores remotos?):

public bool IsAdmin(Localuser user); // Just call private method
public bool IsAdmin(RemoteUser user); // Check if setting is on, then call private method
private bool IsAdmin(int id, string name, string password);

Além disso, se por algum motivo você precisar tornar público o método privado ... você pode. É tão fácil quanto mudar privatepara public. Pode não parecer uma grande vantagem para este caso, mas às vezes é realmente.

Além disso, ao testar a unidade, você poderá fazer muito mais testes atômicos. Geralmente, é muito bom não ter que criar um objeto de usuário completo para executar testes nesse método. E se você quiser cobertura total, poderá testar as duas chamadas.

public bool IsAdmin(int id, string name, string password);

é ruim pelos motivos listados. Isto não significa

public bool IsAdmin(User user);

é automaticamente bom. Em particular se o usuário é um objeto com métodos como: getOrders(), getFriends(), getAdresses(), ...

Você pode refatorar o domínio com um tipo de UserCredentials contendo apenas ID, nome, senha e passá-lo para o IsAdmin, em vez de todos os dados desnecessários do usuário.