Dovresti sempre passare i dati minimi necessari in una funzione in casi come questo

Personalmente preferisco il primo metodo di solo IsAdmin(User user)

È molto più semplice da utilizzare e, se i criteri per IsAdmin vengono modificati in un secondo momento (magari in base ai ruoli o isActive), non è necessario riscrivere la firma del metodo ovunque.

Probabilmente è anche più sicuro in quanto non stai pubblicizzando quali proprietà determinano se un utente è un amministratore o no, o passando per la proprietà della password ovunque. E syrion fa un buon punto che cosa succede quando il tuo id non corrisponde a name / password ?

La lunghezza del codice all'interno di una funzione non dovrebbe davvero importare se il metodo fa il suo lavoro, e preferirei avere un codice di applicazione più breve e più semplice del codice del metodo di supporto.

La prima firma è superiore perché consente l'incapsulamento della logica correlata all'utente nell'oggetto User stesso. Non è utile avere una logica per la costruzione della tupla "id, name, password" sulla tua base di codice. Inoltre, complica la funzione isAdmin : cosa succede se qualcuno passa in una id che non corrisponde alla name , ad esempio? Che cosa succede se si desidera verificare se un determinato utente è un amministratore da un contesto in cui non dovrebbe conoscere la propria password?

Inoltre, come nota sul terzo punto a favore dello stile con argomenti aggiuntivi; può risultare in "meno codice all'interno della funzione", ma dove va quella logica? Non può semplicemente svanire! Invece, si sviluppa su ogni singolo posto in cui viene chiamata la funzione. Per salvare cinque righe di codice in un unico luogo, hai pagato cinque righe per ogni utilizzo della funzione!

Il primo, ma non (solo) per le ragioni che altri hanno dato.

public bool IsAdmin(User user);

Questo è sicuro. Specialmente dal momento che l'Utente è un tipo che hai definito te stesso, c'è poca o nessuna opportunità di trasporre o cambiare argomento. Funziona chiaramente sugli utenti e non è una funzione generica che accetta int e due stringhe. Questa è una parte importante dell'uso di un linguaggio sicuro come Java o C # come appare il tuo codice. Se stai chiedendo informazioni su una lingua specifica, potresti voler aggiungere un tag per quella lingua alla tua domanda.

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

Qui, qualsiasi int e due stringhe faranno. Cosa ti impedisce di trasporre nome e password? Il secondo è più lavoro da usare e consente maggiori possibilità di errore.

Presumibilmente, entrambe le funzioni richiedono che vengano chiamati user.getId (), user.getName () e user.getPassword (), all'interno della funzione (nel primo caso) o prima di chiamare la funzione (nel secondo) quindi la quantità di accoppiamento è la stessa in entrambi i casi. In realtà, poiché questa funzione è valida solo per gli utenti, in Java eliminerei tutti gli argomenti e rendere questo un metodo di istanza sugli oggetti User:

user.isAdmin();

Poiché questa funzione è già strettamente associata agli utenti, ha senso renderla parte di ciò che è un utente.

P.S. Sono sicuro che questo è solo un esempio, ma sembra che tu stia memorizzando una password. Dovresti invece archiviare solo un hash di password crittograficamente sicuro. Durante l'accesso, la password fornita deve essere sottoposta a hash nello stesso modo e confrontata con l'hash memorizzato. L'invio di password in chiaro deve essere evitato. Se si viola questa regola e admmin (int Str Str) dovevano registrare il nome utente, quindi se si è trasferito il nome e la password nel codice, è possibile registrare la password invece che crea un problema di sicurezza (non scrivere password per i registri ) ed è un altro argomento a favore del passaggio di un oggetto invece delle sue parti componenti (o utilizzando un metodo di classe).

Se la tua lingua preferita non supera le strutture in base al valore, la variante (User user) sembra migliore. Che cosa succede se un giorno decidi di cancellare il nome utente e utilizzare solo l'ID / password per identificare l'utente?

Inoltre, questo approccio porta inevitabilmente a chiamate di metodo lunghe o esagerate o incongruenze. Passare 3 argomenti va bene? Che ne dici di 5? O 6? Perché pensarlo, se passare un oggetto è economico in termini di risorse (anche più economico di passare 3 argomenti, probabilmente)?

E io (sorta) non sono d'accordo sul fatto che il secondo approccio dia al lettore più informazioni - intuitivamente, la chiamata al metodo chiede "se l'utente ha diritti di amministratore", non "se la combinazione id / nome / password data ha diritti di amministratore ".

Quindi, se il passaggio dell'oggetto User non comporterebbe la copia di una struttura di grandi dimensioni, il primo approccio è per me più pulito e logico.

Probabilmente avrei entrambi. Il primo come API esterna, con qualche speranza di stabilità nel tempo. Il secondo come un'implementazione privata chiamata dall'API esterna.

Se in un secondo momento dovessi cambiare la regola di controllo, scriverei semplicemente una nuova funzione privata con una nuova firma chiamata da quella esterna.

Questo ha il vantaggio della facilità di cambiare l'implementazione interna. Inoltre è normale che a un certo momento potresti aver bisogno di entrambe le funzioni disponibili contemporaneamente e chiamare l'una o l'altra in base a qualche cambiamento di contesto esterno (ad esempio potresti aver coesistito vecchi utenti e nuovi utenti con campi diversi).

Per quanto riguarda il titolo della domanda, in un modo in entrambi i casi si stanno fornendo le informazioni minime necessarie. Un utente sembra essere la più piccola struttura di dati correlati all'applicazione che contiene le informazioni. I tre campi id, password e nome sembrano essere i più piccoli dati di implementazione effettivamente necessari, ma questi non sono realmente oggetti a livello di applicazione.

In altre parole Se avessi a che fare con un database, un utente sarebbe un record, mentre id, password e login sarebbero campi in quel record.

C'è un punto negativo per inviare classi (tipi di riferimento) ai metodi, e cioè Vulnerabilità di assegnazione di massa . Immagina che invece del metodo IsAdmin(User user) , ho il metodo UpdateUser(User user) .

Se la classe User ha una proprietà booleana chiamata IsAdmin , e se non la controllo durante l'esecuzione del mio metodo, allora sono vulnerabile all'assegnazione di massa. GitHub è stato attaccato proprio da questa firma nel 2012.

Vorrei seguire questo approccio:

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

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

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

• L'utilizzo del tipo di interfaccia come parametro per questa funzione consente di passare oggetti User, per definire oggetti fittizi User per il test e offre una maggiore flessibilità sia sull'uso che sulla manutenzione di questa funzione.

• Ho anche aggiunto la parola chiave this per rendere la funzione un metodo di estensione di tutte le classi di IUser; in questo modo è possibile scrivere codice in modo più intuitivo e utilizzare questa funzione nelle query di Linq. Sarebbe ancora più semplice definire questa funzione in IUser e User, ma presumo che ci sia qualche ragione per cui hai deciso di mettere questo metodo al di fuori di quella classe?

• Uso l'interfaccia personalizzata IID anziché int in quanto ciò consente di ridefinire le proprietà del tuo ID; per esempio. nel caso avessi bisogno di cambiare da int a long , Guid , o qualcos'altro. Probabilmente è esagerato, ma sempre buono per cercare di rendere le cose flessibili in modo tale da non essere bloccati in decisioni precoci.

• NB: l'oggetto IUser può trovarsi in un diverso spazio dei nomi e / o assembly alla classe User, quindi hai la possibilità di mantenere il tuo Codice utente completamente separato dal tuo codice IsAdmin, con loro solo condividendo una libreria referenziata . Esattamente quello che fai c'è una chiamata per come sospetti che questi oggetti siano usati.

Disclaimer:

Per la mia risposta, mi concentrerò sul problema di stile e dimenticherò di un ID, login e password semplice è un buon set di parametri da usare, dal punto di vista della sicurezza. Dovresti essere in grado di estrapolare la mia risposta a qualsiasi insieme di dati di base che stai utilizzando per capire i privilegi dell'utente ...

Considero anche user.isAdmin() equivalente a IsAdmin(User user) . Questa è una scelta che puoi fare.

Rispondi:

Il mio consiglio è solo per l'utente:

public bool IsAdmin(User user);

O usa entrambi, sulla falsariga di:

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

Motivi per il metodo pubblico:

Quando scrivi metodi pubblici di solito è una buona idea pensare a come verranno usati.

In questo caso particolare, il motivo per cui di solito si chiama questo metodo è capire se un determinato utente è un amministratore. Questo è un metodo che ti serve. Davvero non vuoi che ogni chiamante debba scegliere i parametri corretti da inviare (e rischiare errori a causa della duplicazione del codice).

Motivi per il metodo privato:

Il metodo privato che riceve solo il set minimo di parametri richiesti può essere una buona cosa a volte. A volte non così tanto.

Uno degli aspetti positivi della divisione di questi metodi è che è possibile chiamare la versione privata da diversi metodi pubblici nella stessa classe. Ad esempio, se vuoi (per qualsiasi motivo) avere una logica leggermente diversa per Localuser e RemoteUser (forse c'è un'impostazione per attivare / disattivare gli amministratori remoti?):

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

Inoltre, se per qualsiasi motivo hai bisogno di rendere pubblico il metodo privato ... puoi farlo. È facile come cambiare semplicemente private in public . Potrebbe non sembrare un grosso vantaggio per questo caso, ma a volte lo è davvero.

Inoltre, durante i test unitari sarai in grado di effettuare molti più test atomici. Di solito è molto bello non dover creare un oggetto utente completo per eseguire test su questo metodo. E se vuoi una copertura completa, puoi testare entrambe le chiamate.

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

è negativo per i motivi elencati. Ciò non significa

public bool IsAdmin(User user);

è automaticamente buono. In particolare se Utente è un oggetto che ha metodi come: getOrders() , getFriends() , getAdresses() , ...

Potresti prendere in considerazione il refactoring del dominio con un tipo UserCredentials contenente solo ID, nome, password e passaggio a IsAdmin invece di tutti i dati utente non necessari.