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

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

É muito mais fácil de usar e, se seus critérios para IsAdmin forem alterados em uma data posterior (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 está anunciando quais propriedades determinam se um usuário é Administrador ou não, ou passando a propriedade de senha por toda parte. E syrion faz questão de saber o que acontece quando o id não corresponde ao name / password ?

O comprimento do código dentro de uma função não deve importar, desde que o método faça o seu trabalho, e eu prefiro ter um código de aplicação mais curto e 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 próprio objeto User . Não é vantajoso ter lógica para a construção da tupla "id, name, password" sobre sua base de código. Além disso, complica a função isAdmin : o que acontece se alguém passar um id que não corresponda 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 nota em seu terceiro ponto em favor do estilo com argumentos extras; pode resultar em "menos código dentro da função", mas onde é que essa lógica vai? Não pode simplesmente desaparecer! Em vez disso, ele se espalha em torno de cada lugar onde a função é chamada. Para salvar cinco linhas de código em um único lugar, você pagou cinco linhas por uso da função!

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

public bool IsAdmin(User user);

Este é o tipo seguro. Especialmente desde que o usuário é um tipo que você definiu, há pouca ou nenhuma oportunidade para transpor ou trocar argumentos. Ele opera claramente em Usuários e não é uma função genérica que aceita qualquer int e duas strings. Essa é uma grande parte do ponto de usar uma linguagem de tipos seguros, como Java ou C #, com o qual seu código se parece. Se você estiver perguntando sobre um idioma específico, convém 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 para usar e permite mais oportunidades de erro.

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

user.isAdmin();

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

P.S. Tenho certeza de que este é apenas um exemplo, mas parece que você está armazenando uma senha. Você deve armazenar apenas um hash de senha criptograficamente seguro. Durante o login, a senha fornecida deve ser dividida da mesma maneira e comparada com o hash armazenado. O envio de senhas em texto simples deve ser evitado. Se você violar esta regra e isAdmin (int Str Str) registrar o nome de usuário, se você tiver transposto o nome e a senha em seu código, a senha poderá ser registrada, o que cria um problema de segurança (não grave senhas nos registros ) e é outro argumento em favor da passagem de um objeto em vez de suas partes componentes (ou usando um método de classe).

Se o idioma de sua escolha não ultrapassar as estruturas por valor, então (User user) variant parece melhor. E se algum dia você decidir eliminar o nome de usuário e usar o ID / senha para identificar o usuário?

Além disso, essa abordagem inevitavelmente leva a chamadas de método longas ou exageradas ou a inconsistências. Está passando 3 argumentos bem? Como cerca de 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 (de certa forma) discordo que a segunda abordagem dá ao leitor mais informações - intuitivamente, a chamada do método está perguntando "se o usuário tem direitos de administrador", não "se a combinação id / nome / senha dada tem direitos de administrador ".

Portanto, a menos que passar o objeto User resultaria na cópia de uma estrutura grande, a primeira abordagem é mais clara e 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 eu tiver que mudar a regra de verificação, eu apenas escreveria uma nova função privada com uma nova assinatura chamada pelo externo.

Isso tem a vantagem da facilidade de alterar a implementação interna. Também é muito comum que em algum momento você precise das duas funções disponíveis ao mesmo tempo e chame 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).

Com relação ao título da pergunta, de certa forma, em ambos os 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, password e name parecem ser os menores dados de implementação realmente necessários, mas estes não são realmente objetos de nível de aplicação.

Em outras palavras, se você estivesse lidando com um banco de dados, um usuário seria um registro, enquanto id, senha e login seriam campos nesse registro.

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

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

Eu escolheria essa abordagem:

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

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

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

• Usar o tipo de interface como um parâmetro para essa função permite que você passe objetos Usuário, defina objetos do 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-chave this para tornar a função um método de extensão de todas as classes IUser; Desta forma, você pode escrever código de uma maneira mais amigável ao OO e usar essa função em consultas Linq. Mais simples ainda seria definir essa função em IUser e User, mas presumo que haja alguma razão pela qual você decidiu colocar esse método fora dessa classe?

• Eu uso a interface personalizada IID em vez de int , pois isso permite que você redefina as propriedades do seu ID; por exemplo. Se você precisar mudar de int para long , Guid ou outra coisa. Isso é provavelmente um exagero, mas é sempre bom tentar tornar as coisas flexíveis de tal forma que você não esteja preso a decisões precoces.

• NB: Seu objeto IUser pode estar em um namespace e / ou assembly diferentes para a classe User, então você tem a opção de manter seu código User completamente separado de seu código IsAdmin, compartilhando apenas uma biblioteca referenciada . Exatamente o que você faz lá é uma chamada de como você suspeita que esses itens sejam usados.

Exoneração de responsabilidade:

Para minha resposta, vou me concentrar no assunto do estilo e esquecer se um ID, login e senha simples é um bom conjunto de parâmetros para usar, do ponto de vista da segurança. Você deve ser capaz de extrapolar minha resposta para qualquer conjunto básico de dados que esteja usando para descobrir os privilégios do usuário ...

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

Resposta:

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

public bool IsAdmin(User user);

Ou use os dois, nos moldes 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 ideia pensar em como eles serão usados.

Para este caso em particular, a razão pela qual você costuma chamar este 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 cometer 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 boa coisa às vezes. Às vezes nem tanto.

Uma das vantagens de dividir esses métodos é que você pode chamar a versão privada de vários métodos públicos na mesma classe. Por exemplo, se você quisesse (por qualquer motivo) ter lógica ligeiramente diferente para Localuser e 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 o método privado público ... você pode. É tão fácil quanto apenas alterar private para public . Pode não parecer uma grande vantagem para este caso, mas às vezes realmente é.

Além disso, quando estiver testando unidades, 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, pode testar as duas chamadas.

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

é ruim pelas razões listadas. Isso não significa

public bool IsAdmin(User user);

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

Você pode considerar a refatoração do domínio com um tipo UserCredentials contendo apenas id, nome, senha e passando isso para IsAdmin, em vez de todos os dados desnecessários do usuário.