Sollten Sie in solchen Fällen immer das Nötigste an Daten an eine Funktion übergeben?

Angenommen, ich habe eine Funktion IsAdmin, die überprüft, ob ein Benutzer ein Administrator ist. Nehmen wir auch an, dass die Administratorüberprüfung durchgeführt wird, indem Benutzer-ID, Name und Kennwort mit einer Regel abgeglichen werden (nicht wichtig).

In meinem Kopf gibt es dann zwei mögliche Funktionssignaturen dafür:

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

Am häufigsten greife ich zur zweiten Art der Unterschrift und denke, dass:

• Die Funktionssignatur gibt dem Leser viel mehr Informationen
• Die in der Funktion enthaltene Logik muss nichts über die UserKlasse wissen
• Es führt normalerweise zu etwas weniger Code in der Funktion

Allerdings stelle ich diesen Ansatz manchmal in Frage und stelle auch fest, dass er irgendwann unhandlich wird. Wenn zum Beispiel eine Funktion zwischen zehn verschiedenen Objektfeldern in einen resultierenden Bool abbilden würde, würde ich natürlich das gesamte Objekt einschicken. Aber abgesehen von einem so krassen Beispiel sehe ich keinen Grund, das eigentliche Objekt weiterzugeben.

Ich würde mich über Argumente für beide Stile sowie über allgemeine Beobachtungen freuen, die Sie anbieten könnten.

Ich programmiere sowohl in objektorientierten als auch in funktionalen Stilen, daher sollte die Frage in Bezug auf alle Redewendungen gesehen werden.

Ich persönlich bevorzuge die erste Methode von nur IsAdmin(User user)

Es ist viel einfacher zu verwenden, und wenn sich Ihre Kriterien für IsAdmin zu einem späteren Zeitpunkt ändern (möglicherweise basierend auf Rollen oder isActive), müssen Sie Ihre Methodensignatur nicht überall neu schreiben.

Es ist wahrscheinlich auch sicherer, da Sie nicht bekannt geben, welche Eigenschaften bestimmen, ob ein Benutzer ein Administrator ist oder nicht, oder die Kennworteigenschaft überall weitergeben. Und syrion macht einen guten Punkt, dass was passiert, wenn Ihr idnicht mit dem name/ übereinstimmt password?

Die Länge des Codes in einer Funktion sollte eigentlich keine Rolle spielen, wenn die Methode ihre Aufgabe erfüllt, und ich hätte lieber einen kürzeren und einfacheren Anwendungscode als einen Hilfsmethodencode.

Die erste Signatur ist überlegen, da sie die Kapselung benutzerbezogener Logik im UserObjekt selbst ermöglicht. Es ist nicht vorteilhaft, eine Logik für die Erstellung des Tupels "id, name, password" zu haben, das über Ihre Codebasis verstreut ist. Darüber hinaus erschwert es die isAdminFunktion: Was passiert, wenn jemand zum Beispiel ein idnicht passendes Passwort eingibt name? Was ist, wenn Sie überprüfen möchten, ob ein bestimmter Benutzer ein Administrator aus einem Kontext ist, in dem Sie dessen Kennwort nicht kennen sollten?

Außerdem als Anmerkung zu Ihrem dritten Punkt zugunsten des Stils mit zusätzlichen Argumenten; es kann zu "weniger Code in der Funktion" führen, aber wohin geht diese Logik? Es kann nicht einfach verschwinden! Stattdessen ist es auf jeden einzelnen Ort verteilt, an dem die Funktion aufgerufen wird. Um fünf Codezeilen an einem Ort zu speichern, haben Sie fünf Zeilen pro Verwendung der Funktion bezahlt!

Die erste, aber nicht (nur) aus den Gründen, die andere angegeben haben.

public bool IsAdmin(User user);

Dies ist typsicher. Insbesondere da User ein Typ ist, den Sie selbst definiert haben, gibt es kaum oder keine Möglichkeit, Argumente zu transponieren oder zu vertauschen. Es kann eindeutig auf Benutzer angewendet werden und ist keine generische Funktion, die int und zwei Zeichenfolgen akzeptiert. Dies ist ein wichtiger Punkt bei der Verwendung einer typsicheren Sprache wie Java oder C #, wie Ihr Code aussieht. Wenn Sie nach einer bestimmten Sprache fragen, möchten Sie Ihrer Frage möglicherweise ein Tag für diese Sprache hinzufügen.

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

Hier genügen int und zwei Zeichenketten. Was hindert Sie daran, den Namen und das Passwort zu übertragen? Die zweite Methode ist aufwändiger und bietet mehr Möglichkeiten für Fehler.

Vermutlich erfordern beide Funktionen, dass user.getId (), user.getName () und user.getPassword () entweder innerhalb der Funktion (im ersten Fall) oder vor dem Aufrufen der Funktion (im zweiten Fall) aufgerufen werden Das Ausmaß der Kopplung ist in beiden Fällen gleich. Da diese Funktion nur für Benutzer gültig ist, würde ich in Java alle Argumente entfernen und diese Methode zu einer Instanz für Benutzerobjekte machen:

user.isAdmin();

Da diese Funktion bereits eng mit den Benutzern verknüpft ist, ist es sinnvoll, sie zu einem Teil dessen zu machen, was ein Benutzer ist.

PS Ich bin sicher, dass dies nur ein Beispiel ist, aber es sieht so aus, als würden Sie ein Passwort speichern. Sie sollten stattdessen nur einen kryptografisch sicheren Passwort-Hash speichern. Während der Anmeldung sollte das angegebene Passwort auf die gleiche Weise gehasht und mit dem gespeicherten Hash verglichen werden. Das Versenden von Passwörtern im Klartext sollte vermieden werden. Wenn Sie gegen diese Regel verstoßen und isAdmin (int Str Str) den Benutzernamen protokollieren soll, wird möglicherweise stattdessen das Kennwort protokolliert, wodurch ein Sicherheitsproblem entsteht (schreiben Sie keine Kennwörter in die Protokolle) ) und ist ein weiteres Argument für die Übergabe eines Objekts anstelle seiner Bestandteile (oder die Verwendung einer Klassenmethode).

Wenn Ihre gewählte Sprache Strukturen nicht nach Wert (User user)übergibt, scheint die Variante besser zu sein. Was ist, wenn Sie sich eines Tages dazu entschließen, den Benutzernamen zu entfernen und nur die ID / das Passwort zur Identifizierung des Benutzers zu verwenden?

Außerdem führt dieser Ansatz zwangsläufig zu langen und überlasteten Methodenaufrufen oder Inkonsistenzen. Sind 3 Argumente in Ordnung? Wie wäre es mit 5? Oder 6? Warum darüber nachdenken, wenn das Übergeben eines Objekts in Bezug auf Ressourcen billig ist (wahrscheinlich sogar billiger als das Übergeben von drei Argumenten)?

Und ich bin (irgendwie) anderer Meinung, dass der zweite Ansatz dem Leser mehr Informationen liefert - intuitiv fragt der Methodenaufruf "ob der Benutzer Administratorrechte hat", nicht "ob die angegebene Kombination aus ID, Name und Passwort Administratorrechte hat".

Wenn das Übergeben des UserObjekts nicht zum Kopieren einer großen Struktur führen würde, ist der erste Ansatz für mich klarer und logischer.

Ich hätte wahrscheinlich beides. Die erste als externe API mit Hoffnung auf Stabilität in der Zeit. Die zweite als private Implementierung, die von der externen API aufgerufen wird.

Wenn ich zu einem späteren Zeitpunkt die Prüfregel ändern muss, würde ich einfach eine neue private Funktion mit einer neuen Signatur schreiben, die von der externen aufgerufen wird.

Dies hat den Vorteil, dass die innere Implementierung leicht geändert werden kann. Es ist auch durchaus üblich, dass Sie manchmal beide Funktionen gleichzeitig benötigen und abhängig von einer Änderung des externen Kontexts die eine oder andere aufrufen müssen (z. B. können alte Benutzer und neue Benutzer mit unterschiedlichen Feldern gleichzeitig vorhanden sein).

In Bezug auf den Titel der Frage geben Sie in beiden Fällen die minimal erforderlichen Informationen an. Ein Benutzer scheint die kleinste anwendungsbezogene Datenstruktur zu sein, die die Informationen enthält. Die drei Felder id, password und name scheinen die kleinsten Implementierungsdaten zu sein, die tatsächlich benötigt werden, es handelt sich jedoch nicht wirklich um Objekte auf Anwendungsebene.

Mit anderen Worten, wenn Sie sich mit einer Datenbank befassen, wäre ein Benutzer ein Datensatz, während ID, Kennwort und Anmeldung Felder in diesem Datensatz wären.

Es gibt einen negativen Punkt beim Senden von Klassen (Referenztypen) an Methoden, nämlich die Sicherheitsanfälligkeit bezüglich Massenzuweisung . Stellen Sie sich vor, dass IsAdmin(User user)ich anstelle einer Methode eine UpdateUser(User user)Methode habe.

Wenn Userclass eine boolesche Eigenschaft namens IsAdminhat und ich sie während der Ausführung meiner Methode nicht überprüfe, bin ich anfällig für Massenzuweisungen. GitHub wurde 2012 von genau dieser Signatur angegriffen.

Ich würde mit diesem Ansatz gehen:

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

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

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

• Die Verwendung des Schnittstellentyps als Parameter für diese Funktion ermöglicht die Übergabe von Benutzerobjekten, die Definition von nachgebildeten Benutzerobjekten zum Testen und bietet Ihnen mehr Flexibilität bei der Verwendung und Wartung dieser Funktion.

• Ich habe auch das Schlüsselwort hinzugefügt this, um die Funktion zu einer Erweiterungsmethode aller IUser-Klassen zu machen. Auf diese Weise können Sie OO-freundlichen Code schreiben und diese Funktion in Linq-Abfragen verwenden. Noch einfacher wäre es, diese Funktion in IUser und User zu definieren, aber ich nehme an, dass es einen Grund gibt, warum Sie sich entschieden haben, diese Methode außerhalb dieser Klasse zu platzieren?

• IIDStattdessen verwende ich die benutzerdefinierte Benutzeroberfläche, damit intSie die Eigenschaften Ihrer ID neu definieren können. zB sollten Sie jemals von ändern müssen intzu long, Guidoder etwas anderes. Das ist wahrscheinlich übertrieben, aber es ist immer gut zu versuchen, die Dinge so flexibel zu gestalten, dass Sie nicht an frühzeitige Entscheidungen gebunden sind.

• Hinweis: Ihr IUser-Objekt kann sich in einem anderen Namespace und / oder einer anderen Assembly als die User-Klasse befinden. Sie haben daher die Möglichkeit, Ihren Benutzercode vollständig von Ihrem IsAdmin-Code zu trennen, sodass nur eine referenzierte Bibliothek gemeinsam genutzt wird. Genau das, was Sie dort tun, ist eine Aufforderung, wie Sie vermuten, dass diese Gegenstände verwendet werden.

Haftungsausschluss:

In meiner Antwort werde ich mich auf das Stilproblem konzentrieren und vergessen, ob eine ID, ein Login und ein einfaches Passwort aus Sicherheitsgründen eine gute Auswahl an Parametern sind. Sie sollten in der Lage sein, meine Antwort auf alle grundlegenden Daten zu extrapolieren, die Sie verwenden, um die Berechtigungen des Benutzers herauszufinden ...

Ich halte user.isAdmin()das auch für äquivalent zu IsAdmin(User user). Das ist eine Entscheidung, die Sie treffen müssen.

Antworten:

Meine Empfehlung gilt nur für beide Benutzer:

public bool IsAdmin(User user);

Oder verwenden Sie beide nach folgenden Grundsätzen:

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

Gründe für die öffentliche Methode:

Wenn Sie öffentliche Methoden schreiben, ist es normalerweise eine gute Idee, darüber nachzudenken, wie sie verwendet werden.

In diesem speziellen Fall wird diese Methode normalerweise aufgerufen, um herauszufinden, ob ein bestimmter Benutzer ein Administrator ist. Das ist eine Methode, die Sie brauchen. Sie möchten wirklich nicht, dass jeder Anrufer die richtigen Parameter zum Senden auswählen muss (und riskieren Fehler aufgrund von Code-Duplikationen).

Gründe für die private Methode:

Die private Methode, die nur den Mindestsatz der erforderlichen Parameter empfängt, kann manchmal eine gute Sache sein. Manchmal nicht so sehr.

Das Aufteilen dieser Methoden hat den Vorteil, dass Sie die private Version von mehreren öffentlichen Methoden in derselben Klasse aufrufen können. Zum Beispiel, wenn Sie (aus welchem ​​Grund auch immer) eine etwas andere Logik für Localuserund RemoteUser(vielleicht gibt es eine Einstellung zum Ein- / Ausschalten von Remote-Administratoren?) Haben wollten :

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

Wenn Sie aus irgendeinem Grund die private Methode öffentlich machen müssen, können Sie dies auch. Es ist so einfach wie nur die Änderung privatean public. Es mag für diesen Fall nicht so großartig erscheinen, aber manchmal ist es wirklich so.

Außerdem können Sie beim Testen von Einheiten wesentlich mehr Atomtests durchführen. Es ist normalerweise sehr schön, kein vollständiges Benutzerobjekt erstellen zu müssen, um Tests mit dieser Methode auszuführen. Wenn Sie eine vollständige Abdeckung wünschen, können Sie beide Anrufe testen.

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

ist aus den genannten Gründen schlecht. Das heißt nicht

public bool IsAdmin(User user);

ist automatisch gut. Insbesondere dann , wenn Benutzer eine Objektmethode, die wie: getOrders(), getFriends(), getAdresses(), ...

Sie können die Domäne mit einem UserCredentials-Typ umgestalten, der nur die ID, den Namen und das Kennwort enthält, und diesen anstelle aller nicht benötigten Benutzerdaten an IsAdmin übergeben.