So implementieren Sie eine Eigenschaft in Klasse A, die sich auf eine Eigenschaft eines untergeordneten Objekts der Klasse A bezieht

9

Wir haben diesen Code, der vereinfacht so aussieht:

public class Room
{
    public Client Client { get; set; }

    public long ClientId
    {
        get
        {
            return Client == null ? 0 : Client.Id;
        }
    }
}

public class Client 
{
    public long Id { get; set; }
}

Jetzt haben wir drei Standpunkte.

1) Dies ist ein guter Code, da die ClientEigenschaft immer festgelegt werden sollte (dh nicht null), damit Client == nullsie niemals auftritt und der ID-Wert 0ohnehin eine falsche ID angibt (dies ist die Meinung des Verfassers des Codes ;-))

2) Sie können nicht auf den Anrufer verlassen , um zu wissen , dass 0ein falscher Wert für Idund wenn die ClientEigenschaft immer eingestellt werden sollte , sollten Sie eine werfen exceptionin der , getwenn die ClientEigenschaft null sein geschieht

3) Wenn die ClientEigenschaft immer festgelegt werden soll, kehren Sie einfach zurück Client.Idund lassen den Code eine NullRefAusnahme Clientauslösen, wenn die Eigenschaft zufällig null ist.

Welche davon ist am richtigsten? Oder gibt es eine vierte Möglichkeit?

Michel
quelle
5
Keine Antwort, sondern nur eine Anmerkung: Werfen Sie niemals Ausnahmen von Immobilien-Gettern.
Brandon
3
Um auf Brandons Punkt einzugehen
MetaFight
1
Sicher: Die allgemeine Idee ( unter anderem msdn.microsoft.com/en-us/library/… , stackoverflow.com/questions/1488472/… ) ist, dass Eigenschaften so wenig wie möglich tun, leicht sein und die Sauberkeit darstellen sollten , öffentlich zugänglicher Status Ihres Objekts, wobei Indexer eine geringfügige Ausnahme darstellen. Es ist auch ein unerwartetes Verhalten, während des Debuggens Ausnahmen in einem Überwachungsfenster für eine Eigenschaft anzuzeigen.
Brandon
1
Sie sollten (müssen) keinen Code schreiben, der einen Property Getter einwirft. Das bedeutet nicht, dass Sie Ausnahmen verbergen und verschlucken müssen, die auftreten, wenn ein Objekt in einen nicht unterstützten Zustand versetzt wird.
Ben
4
Warum kannst du nicht einfach Room.Client.Id machen? Warum möchten Sie das in Room.ClientId einbinden? Wenn Sie nach einem Client suchen möchten, warum dann nicht so etwas wie Room.HasClient {get {return client == null; }} das ist einfacher und lässt die Leute trotzdem den normalen Room.Client.Id machen, wenn sie tatsächlich die ID brauchen?
James

Antworten:

25

Es riecht so, als sollten Sie die Anzahl der Zustände begrenzen, in denen sich Ihre RoomKlasse befinden kann.

Die Tatsache, dass Sie fragen, was zu tun ist, wenn Clientnull ist, ist ein Hinweis darauf, dass Roomder Statusraum zu groß ist.

Um die Dinge einfach zu halten, würde ich nicht zulassen, dass die ClientEigenschaft einer RoomInstanz jemals null ist. Das bedeutet, dass der darin enthaltene Code Roomsicher annehmen kann, dass der Clientniemals null ist.

Wenn aus irgendeinem Grunde in der Zukunft Clientwird nullden Drang widerstehen , diesen Zustand zu unterstützen. Dies erhöht Ihre Wartungskomplexität.

Lassen Sie stattdessen zu, dass der Code fehlschlägt und schnell fehlschlägt. Dies ist schließlich kein unterstützter Zustand. Wenn sich die Anwendung in diesen Zustand versetzt, haben Sie bereits eine Linie ohne Rückgabe überschritten. Das einzig Vernünftige ist dann, die Anwendung zu schließen.

Dies kann (ganz natürlich) als Ergebnis einer nicht behandelten Nullreferenzausnahme geschehen.

MetaFight
quelle
2
Nett. Ich mag den Gedanken "Um die Dinge einfach zu halten, würde ich nicht zulassen, dass die Client-Eigenschaft einer Room-Instanz jemals null ist." Das ist in der Tat besser, als zu fragen, was zu tun ist, wenn es null ist
Michel
@Michel Wenn Sie sich später dazu entschließen, diese Anforderung zu ändern, können Sie den nullWert durch einen NullObject(oder besser NullClient) ersetzen, der dann weiterhin mit Ihrem vorhandenen Code funktioniert, und bei Bedarf das Verhalten für einen nicht vorhandenen Client definieren. Die Frage ist, ob der Fall "kein Client" Teil der Geschäftslogik ist oder nicht.
null
3
Völlig richtig. Schreiben Sie kein einziges Codezeichen, um einen nicht unterstützten Status zu unterstützen. Lass es einfach ein NullRef werfen.
Ben
@ Ben nicht einverstanden; In den MS-Richtlinien wird ausdrücklich festgelegt, dass Property Getter keine Ausnahmen auslösen sollten. Und das Auslösen von Null-Refs, wenn stattdessen eine aussagekräftigere Ausnahme ausgelöst werden könnte, ist nur faul.
Andy
1
Wenn Sie den Grund für die Richtlinie verstehen, können Sie feststellen, wann sie nicht zutrifft.
Ben
21

Nur ein paar Überlegungen:

a) Warum gibt es einen Getter speziell für die ClientId, wenn es bereits einen öffentlichen Getter für die Client-Instanz selbst gibt? Ich verstehe nicht, warum die Information, dass es sich bei der ClientId um eine handelt long, in die Signatur von Room in Stein gemeißelt werden muss.

b) In Bezug auf die zweite Meinung könnten Sie eine Konstante einführen Invalid_Client_Id.

c) In Bezug auf Meinung eins und drei (und als mein Hauptpunkt): Ein Raum muss immer einen Kunden haben? Vielleicht ist es nur Semantik, aber das klingt nicht richtig. Vielleicht wäre es angemessener, vollständig getrennte Klassen für Raum und Kunde und eine andere Klasse zu haben, die sie miteinander verbindet. Vielleicht Termin, Reservierung, Belegung? (Dies hängt davon ab, was Sie tatsächlich tun.) Und für diese Klasse können Sie die Einschränkungen "muss einen Raum haben" und "muss einen Kunden haben" durchsetzen.

VolkerK
quelle
5
+1 für "Ich verstehe zB nicht, warum die Information, dass die ClientId lang ist, in die Signatur von Room in Stein gemeißelt werden muss"
Michel
Dein Englisch ist gut. ;) Nur durch das Lesen dieser Antwort hätte ich nicht gewusst, dass Ihre Muttersprache nicht Englisch ist, ohne dass Sie dies sagen.
jpmc26
Die Punkte B & C sind gut; RE: a, es ist eine bequeme Methode, und die Tatsache, dass Raum einen Verweis auf Client hat, könnte nur ein Implementierungsdetail sein (es könnte argumentiert werden, dass Client nicht öffentlich sichtbar sein sollte)
Andy
17

Ich bin mit allen drei Meinungen nicht einverstanden. Wenn Clientes niemals sein kann null, dann mach es nicht einmal möglichnull !

  1. Legen Sie den Wert von Clientim Konstruktor fest
  2. Wirf einen ArgumentNullExceptionin den Konstruktor.

Ihr Code wäre also ungefähr so:

public class Room
{
    private Client theClient;

    public Room(Client client) {
        if(client == null) throw new ArgumentNullException();
        this.theClient = client;
    }

    public Client Client { 
        get { return theClient; }
        set 
        {
            if (value == null) 
                throw new ArgumentNullException();
            theClient = value;
        }
    }

    public long ClientId
    {
        get
        {
            return theClient.Id;
        }
    }
}
public class Client 
{
    public long Id { get; set; }
}

Dies hat nichts mit Ihrem Code zu tun, aber Sie sollten wahrscheinlich eine unveränderliche Version von verwenden, Roomindem Sie machen theClient readonlyund dann, wenn sich der Client ändert, einen neuen Raum erstellen . Dies verbessert die Thread-Sicherheit Ihres Codes zusätzlich zur Nullsicherheit der anderen Aspekte meiner Antwort. Siehe diese Diskussion über veränderlich gegen unveränderlich

durron597
quelle
1
Sollten das nicht ArgumentNullExceptions sein?
Mathieu Guindon
4
Nein. Der Programmcode sollte niemals a auslösen NullReferenceException. Er wird von der .NET-VM ausgelöst, "wenn versucht wird, eine Nullobjektreferenz zu dereferenzieren" . Sie sollten ein auslösen ArgumentNullException, das ausgelöst wird, "wenn eine Nullreferenz (Nothing in Visual Basic) an eine Methode übergeben wird, die sie nicht als gültiges Argument akzeptiert."
Pharap
2
@Pharap Ich bin ein Java-Programmierer, der versucht, C # -Code zu schreiben. Ich dachte, ich würde solche Fehler machen.
Durron597
@ durron597 Ich hätte das aus der Verwendung des Begriffs 'final' erraten sollen (in diesem Fall ist das entsprechende C # -Schlüsselwort readonly). Ich hätte gerade bearbeitet, aber ich hatte das Gefühl, ein Kommentar würde dazu dienen, besser zu erklären, warum (für zukünftige Leser). Wenn Sie diese Antwort nicht bereits gegeben hätten, würde ich genau die gleiche Lösung geben. Unveränderlichkeit ist auch eine gute Idee, aber es ist nicht immer so einfach, wie man hofft.
Pharap
2
Eigenschaften sollten im Allgemeinen keine Ausnahmen auslösen. Geben Sie anstelle eines Setters, der ArgumentNullException auslöst, stattdessen eine SetClient-Methode an. Jemand hat den Link zu einer entsprechenden Antwort auf die Frage gepostet.
Andy
5

Die zweite und dritte Option sollten vermieden werden - der Getter sollte den Anrufer nicht schlagen, mit einer Ausnahme, über die er keine Kontrolle hat.

Sie sollten entscheiden, ob der Client jemals null sein kann. In diesem Fall sollten Sie einem Aufrufer die Möglichkeit geben, vor dem Zugriff zu überprüfen, ob er null ist (z. B. die Eigenschaft bool ClientIsNull).

Wenn Sie entscheiden, dass der Client niemals null sein kann, machen Sie ihn zu einem erforderlichen Parameter für den Konstruktor und lösen Sie dort die Ausnahme aus, wenn eine Null übergeben wird.

Schließlich enthält die erste Option auch einen Codegeruch. Sie sollten den Client mit seiner eigenen ID-Eigenschaft befassen lassen. Es scheint übertrieben, einen Getter in einer Containerklasse zu codieren, die einfach einen Getter für die enthaltene Klasse aufruft. Stellen Sie den Client einfach als Eigenschaft bereit (andernfalls duplizieren Sie am Ende alles, was ein Client bereits anbietet ).

long clientId = room.Client.Id;

Wenn der Client null sein kann, geben Sie dem Anrufer zumindest die Verantwortung:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}
Kent A.
quelle
1
Ich denke, "Sie werden am Ende alles duplizieren, was ein Kunde bereits anbietet" muss fett oder zumindest kursiv sein.
Pharap
2

Wenn eine Null-Client-Eigenschaft unterstützt wird, sollten Sie ein NullObject verwenden .

Aber höchstwahrscheinlich ist dies ein außergewöhnlicher Zustand, daher sollten Sie es unmöglich (oder einfach nicht sehr praktisch) machen, eine Null zu erhalten Client:

public Client Client { get; private set; }

public Room(Client client)
{
    Client = client;
}

public void SetClient(Client client)
{
    if (client == null) throw new ArgumentNullException();
    Client = client;
}

Wenn es jedoch nicht unterstützt wird, verschwenden Sie keine Zeit mit halbgebackenen Lösungen. In diesem Fall:

return Client == null ? 0 : Client.Id;

Sie haben die NullReferenceException hier "gelöst", aber zu einem hohen Preis! Dies würde alle Anrufer von zwingen Room.ClientId, nach 0 zu suchen:

var aRoom = new Room(aClient);
var clientId = aRoom.ClientId;
if (clientId == 0) RestartRoombookingWizard();
else ContinueRoombooking(clientid);

Seltsame Fehler können auftreten, wenn andere Entwickler (oder Sie selbst!) Vergessen, den Rückgabewert "ErrorCode" zu einem bestimmten Zeitpunkt zu überprüfen.
Gehen Sie auf Nummer sicher und scheitern Sie schnell. Lassen Sie die NullReferenceException ausgelöst werden und fangen Sie sie "anmutig" irgendwo höher im Aufrufstapel ab, anstatt dem Aufrufer zu erlauben, die Dinge noch mehr durcheinander zu bringen ...

In einem anderen Sinne , wenn Sie so sehr daran interessiert sind, das Clientund auch das ClientIdDenken über TellDontAsk aufzudecken . Wenn Sie zu viel von Objekten verlangen, anstatt ihnen zu sagen, was Sie erreichen möchten, beenden Sie möglicherweise die Kopplung aller Elemente, wodurch Änderungen später schwieriger werden.

Laoujin
quelle
Das NotSupportedExceptionwird missbraucht, Sie sollten stattdessen eine ArgumentNullExceptionverwenden.
Pharap
1

Ab c # 6.0, das jetzt veröffentlicht wird, sollten Sie dies einfach tun:

public class Room
{
    public Room(Client client)
    {
        this.Client = client;
    }

    public Client Client { get; }
}

public class Client
{
    public Client(long id)
    {
        this.Id = id;
    }

    public long Id { get; }
}

Das Erhöhen einer Eigenschaft von Clientto Roomist ein offensichtlicher Verstoß gegen die Kapselung und das DRY-Prinzip.

Wenn Sie auf das Idvon a Clientvon a zugreifen Roommüssen, können Sie Folgendes tun:

var clientId = room.Client?.Id;

Beachten Sie die Verwendung des Null Conditional Operator , clientIdwird eine sein long?, wenn Clientist null, clientIdwird sein null, sonst clientIdwird der Wert haben Client.Id.

Jodrell
quelle
aber die Art von clientidist dann eine nullable lange?
Michel
@ Michael Nun, wenn ein Raum einen Client haben muss, setzen Sie einen Null-Check in den Ctor. Dann var clientId = room.Client.Idwird sowohl sicher als auch long.
Jodrell