Si siempre pasa la información mínima necesaria a una función en casos como este

Personalmente prefiero el primer método de solo IsAdmin(User user)

Es mucho más fácil de usar, y si sus criterios para IsAdmin cambian en una fecha posterior (tal vez en función de los roles, o es Activo), no necesita volver a escribir la firma de su método en todas partes.

También es probable que sea más seguro, ya que no está anunciando qué propiedades determinan si un usuario es un administrador o no, o si pasa la propiedad de la contraseña a todas partes. Y syrion señala que ¿qué sucede cuando su id no coincide con el name / password ?

La longitud del código dentro de una función no debería importar realmente, ya que el método hace su trabajo, y prefiero tener un código de aplicación más corto y simple que el código del método auxiliar.

La primera firma es superior porque permite la encapsulación de la lógica relacionada con el usuario en el objeto User . No es beneficioso tener lógica para la construcción de la tupla "id, name, password" sobre su base de código. Además, complica la función isAdmin : ¿qué sucede si alguien pasa un id que no coincide con el name , por ejemplo? ¿Qué sucede si desea comprobar si un usuario determinado es un administrador de un contexto en el que no debería conocer su contraseña?

Además, como una nota en su tercer punto a favor del estilo con argumentos adicionales; puede resultar en "menos código dentro de la función", pero ¿a dónde va esa lógica? ¡No puede simplemente desaparecer! En su lugar, se extiende alrededor de cada lugar donde se llama la función. Para guardar cinco líneas de código en un solo lugar, ¡ha pagado cinco líneas por uso de la función!

El primero, pero no (solo) por las razones que otros han dado.

public bool IsAdmin(User user);

Este es un tipo seguro. Especialmente dado que el Usuario es un tipo que usted mismo definió, hay poca o ninguna oportunidad de transponer o cambiar los argumentos. Funciona claramente en Usuarios y no es una función genérica que acepte ninguna cadena int y dos cadenas. Esta es una parte importante del uso de un lenguaje de tipo seguro como Java o C #, como se ve su código. Si está preguntando sobre un idioma específico, es posible que desee agregar una etiqueta para ese idioma a su pregunta.

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

Aquí, cualquier int y dos cadenas harán. ¿Qué te impide transponer el nombre y la contraseña? El segundo es más trabajo para usar y permite más oportunidad de error.

Probablemente, ambas funciones requieren que se llame a user.getId (), user.getName () y user.getPassword (), ya sea dentro de la función (en el primer caso) o antes de llamar a la función (en el segundo) , por lo que la cantidad de acoplamiento es la misma de cualquier manera. En realidad, dado que esta función solo es válida para los usuarios, en Java eliminaría todos los argumentos y haría de este un método de instancia en los objetos del usuario:

user.isAdmin();

Dado que esta función ya está estrechamente unida a los Usuarios, tiene sentido hacer que sea parte de lo que es un usuario.

P.S. Estoy seguro de que esto es solo un ejemplo, pero parece que está almacenando una contraseña. En su lugar, solo debe almacenar un hash de contraseña criptográficamente seguro. Durante el inicio de sesión, la contraseña suministrada se debe cifrar de la misma manera y comparar con el hash almacenado. Debe evitarse el envío de contraseñas en texto plano. Si viola esta regla e isAdmin (int Str Str) registra el nombre de usuario, entonces, si transpuso el nombre y la contraseña en su código, la contraseña podría registrarse en su lugar, lo que crea un problema de seguridad (no escriba contraseñas en los registros). ) y es otro argumento a favor de pasar un objeto en lugar de sus partes componentes (o usar un método de clase).

Si su idioma de elección no pasa las estructuras por valor, entonces la variante (User user) parece mejor. ¿Qué sucede si algún día decide descartar el nombre de usuario y simplemente usar la ID / contraseña para identificar al usuario?

Además, este enfoque conduce inevitablemente a llamadas a métodos largos y exagerados, o inconsistencias. ¿Está pasando 3 argumentos bien? ¿Qué tal 5? O 6? ¿Por qué pensar en eso, si pasar un objeto es barato en términos de recursos (incluso más barato que pasar 3 argumentos, probablemente)?

Y (más o menos) no estoy de acuerdo en que el segundo enfoque le brinde más información al lector: de manera intuitiva, la llamada al método pregunta "si el usuario tiene derechos de administrador", no "si la combinación de id / nombre / contraseña dada tiene derechos de administrador ".

Entonces, a menos que pasar el objeto User resultaría en copiar una estructura grande, el primer enfoque es más limpio y lógico para mí.

Probablemente tendría ambos. El primero como una API externa, con alguna esperanza de estabilidad en el tiempo. El segundo como una implementación privada llamada por la API externa.

Si en algún momento tengo que cambiar la regla de verificación, solo escribiría una nueva función privada con una nueva firma llamada por la externa.

Esto tiene la ventaja de que es fácil cambiar la implementación interna. Además, es muy habitual que en algún momento necesite que ambas funciones estén disponibles al mismo tiempo y llame a una u otra, dependiendo de algún cambio de contexto externo (por ejemplo, puede que coexistan Usuarios antiguos y Usuarios nuevos con campos diferentes).

Con respecto al título de la pregunta, de una manera, en ambos casos, está proporcionando la información mínima necesaria. Un usuario parece ser la estructura de datos relacionada con la aplicación más pequeña que contiene la información. Los tres campos id, contraseña y nombre parecen ser los datos de implementación más pequeños que realmente se necesitan, pero estos no son realmente objetos de nivel de aplicación.

En otras palabras, si estuviera tratando con una base de datos, un Usuario sería un registro, mientras que la identificación, la contraseña y el inicio de sesión serían campos en ese registro.

Hay un punto negativo para enviar clases (tipos de referencia) a los métodos, es decir, Vulnerabilidad en la asignación masiva . Imagina que en lugar del método IsAdmin(User user) , tengo el método UpdateUser(User user) .

Si la clase User tiene una propiedad booleana llamada IsAdmin , y si no la verifico durante la ejecución de mi método, entonces soy vulnerable a la asignación masiva. GitHub fue atacado por esta misma firma en 2012.

Me gustaría con este enfoque:

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

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

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

• El uso del tipo de interfaz como parámetro para esta función le permite pasar objetos de usuario, definir objetos de usuario simulados para realizar pruebas y le brinda más flexibilidad tanto en el uso como en el mantenimiento de esta función.

• También agregué la palabra clave this para hacer de la función un método de extensión de todas las clases de IUser; De esta manera, puede escribir código de una manera más fácil para OO y utilizar esta función en las consultas de Linq. Más sencillo aún sería definir esta función en IUser y en Usuario, pero supongo que hay alguna razón por la que decidió poner este método fuera de esa clase.

• Utilizo la interfaz personalizada IID en lugar de int , ya que esto le permite redefinir las propiedades de su ID; p.ej. si alguna vez necesitas cambiar de int a long , Guid , o algo más. Probablemente eso sea una exageración, pero siempre es bueno tratar de hacer las cosas más flexibles, de modo que no esté atrapado en decisiones tempranas.

• NB: su objeto IUser puede estar en un espacio de nombres y / o ensamblaje diferente al de la clase de usuario, por lo que tiene la opción de mantener su código de usuario completamente separado de su código IsAdmin, y solo comparten una biblioteca a la que se hace referencia . Exactamente lo que haces es una llamada sobre cómo sospechas que se usarán estos elementos.

Disclaimers:

Para mi respuesta, me centraré en el tema del estilo y me olvidaré de si un ID, un inicio de sesión y una contraseña simple son un buen conjunto de parámetros para usar, desde el punto de vista de la seguridad. Debería poder extrapolar mi respuesta a cualquier conjunto básico de datos que esté utilizando para averiguar los privilegios del usuario ...

También considero que user.isAdmin() es equivalente a IsAdmin(User user) . Esa es una elección para que hagas.

Responder :

Mi recomendación es solo para cualquiera de los usuarios:

public bool IsAdmin(User user);

O utilice ambos, en la línea de:

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

Razones para el método público:

Cuando se escriben métodos públicos, generalmente es una buena idea pensar cómo se usarán.

Para este caso en particular, la razón por la que normalmente llamas a este método es para averiguar si un determinado usuario es un administrador. Ese es un método que necesitas. Realmente no desea que cada persona que llama tenga que elegir los parámetros correctos para enviar (y arriesgarse a errores debido a la duplicación de código).

Razones para el método privado:

El método privado que recibe solo el conjunto mínimo de parámetros requeridos puede ser bueno a veces. A veces no tanto.

Una de las cosas buenas de dividir estos métodos es que puede llamar a la versión privada desde varios métodos públicos de la misma clase. Por ejemplo, si quisiera (por cualquier razón) tener una lógica ligeramente diferente para Localuser y RemoteUser (tal vez haya una opción para activar / desactivar los 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);

Además, si por algún motivo necesita hacer público el método privado ... puede hacerlo. Es tan fácil como cambiar private a public . Puede que no parezca una gran ventaja para este caso, pero a veces realmente lo es.

Además, cuando realice pruebas de unidad, podrá realizar muchas más pruebas atómicas. Por lo general, es muy agradable no tener que crear un objeto de usuario completo para ejecutar pruebas en este método. Y si desea una cobertura completa, puede probar ambas llamadas.

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

es malo por las razones enumeradas. Eso no significa

public bool IsAdmin(User user);

es automáticamente bueno. En particular, si Usuario es un objeto que tiene métodos como: getOrders() , getFriends() , getAdresses() , ...

Puede considerar refactorizar el dominio con un tipo de UserCredentials que contenga solo id, nombre, contraseña y pasarlo a IsAdmin en lugar de todos los datos de usuario innecesarios.