Come implementare una proprietà sulla classe A che si riferisce a una proprietà di un oggetto figlio di classe A

9

Abbiamo questo codice che, quando è semplificato, assomiglia a questo:

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

Ora abbiamo tre punti di vista.

1) Questo è un buon codice perché la proprietà Client dovrebbe sempre essere impostata (cioè non nulla) in modo che Client == null non si verifichi mai e il valore Id 0 denota comunque un id falso (questa è l'opinione di lo scrittore del codice; -))

2) Non puoi fare affidamento sul chiamante per sapere che 0 è un valore falso per Id e quando la proprietà Client dovrebbe sempre essere impostata devi lanciare un exception nel get quando la proprietà Client risulta essere null

3) Quando la proprietà Client deve sempre essere impostata, devi solo restituire Client.Id e lasciare che il codice lanci un'eccezione NullRef quando la proprietà Client risulta essere nulla.

Quale di questi è più corretto? O c'è una quarta possibilità?

    
posta Michel 24.08.2015 - 14:31
fonte

6 risposte

25

Sembra che tu debba limitare il numero di stati in cui può trovarsi la tua classe Room .

Il fatto stesso che tu stia chiedendo cosa fare quando Client è nullo è un suggerimento che lo spazio di stato di Room è troppo grande.

Per semplificare le cose non permetterei che la proprietà Client di qualsiasi istanza di Room sia mai nulla. Ciò significa che il codice all'interno di Room può tranquillamente assumere che Client non sia mai nullo.

Se per qualche ragione in futuro Client diventa null resisti all'impulso di supportare quello stato. Così facendo aumenterai la tua complessità di manutenzione.

Invece, consentire al codice di fallire e fallire velocemente. Dopotutto, questo non è uno stato supportato. Se l'applicazione si inserisce in questo stato, hai già superato una linea di non ritorno. L'unica cosa ragionevole da fare è chiudere l'applicazione.

Questo potrebbe accadere (abbastanza naturalmente) come risultato di un'eccezione di riferimento null non gestita.

    
risposta data 24.08.2015 - 15:25
fonte
21

Solo alcune considerazioni:

a) Perché c'è un getter specifico per ClientId quando c'è già un getter pubblico per l'istanza del Cliente stesso? Non vedo perché l'informazione che il ClientId è un long debba essere scolpita nella pietra nella firma di Room.

b) Per quanto riguarda il secondo parere, potresti introdurre una costante Invalid_Client_Id .

c) Riguardo all'opinione 1 e 3 (ed essendo il mio punto principale): una stanza deve sempre avere un cliente? Forse è solo semantica ma questo non suona bene. Forse sarebbe più appropriato avere classi completamente separate per Room e Client e un'altra classe che le lega insieme. Forse appuntamento, prenotazione, occupazione? (Dipende da cosa stai effettivamente facendo). E su quella classe puoi applicare i vincoli "deve avere una stanza" e "deve avere un cliente".

    
risposta data 24.08.2015 - 16:02
fonte
17

Non sono d'accordo con tutte e tre le opinioni. Se Client non può mai essere null , quindi non renderebbe nemmeno possibile che sia null !

  1. Imposta il valore di Client nel costruttore
  2. Lancia un ArgumentNullException nel costruttore.

Quindi il tuo codice sarebbe qualcosa del tipo:

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

Questo non è correlato al tuo codice, ma probabilmente starai meglio usando una versione immutable di Room creando theClient readonly , e poi se il client cambia, facendo un nuova stanza. Ciò migliorerà la sicurezza del thread del codice in aggiunta alla sicurezza nulla degli altri aspetti della mia risposta. Vedi questa discussione su mutable vs immutable

    
risposta data 24.08.2015 - 18:43
fonte
5

La seconda e la terza opzione dovrebbero essere evitate - il getter non dovrebbe schiaffeggiare il chiamante con un'eccezione su cui non hanno alcun controllo.

Dovresti decidere se il Cliente può mai essere nullo. In tal caso, è necessario fornire un modo per consentire a un chiamante di verificare se è nullo prima di accedervi (ad es., Bool di proprietà ClientIsNull).

Se decidi che il Cliente non può mai essere nullo, allora rendilo un parametro obbligatorio per il costruttore e lancia l'eccezione lì se viene passato un nulla.

Infine, la prima opzione contiene anche un odore di codice. È necessario consentire al Cliente di gestire la propria proprietà ID. Sembra eccessivo usare il codice di un getter in una classe contenitore che chiama semplicemente un getter nella classe contenuta. Basta esporre il Cliente come una proprietà (altrimenti, finirai per duplicare tutto ciò che un Client offre già ).

long clientId = room.Client.Id;

Se il Cliente può essere nullo, allora dovrai almeno responsabilizzare il chiamante:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}
    
risposta data 24.08.2015 - 16:05
fonte
2

Se una proprietà Null Client è uno stato supportato, prendere in considerazione l'utilizzo di un NullObject .

Ma molto probabilmente questo è uno stato eccezionale, quindi dovresti rendere impossibile (o semplicemente non molto conveniente) finire con un Client nullo:

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

Tuttavia, se non è supportato, non perdere tempo con soluzioni a metà cottura. In questo caso:

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

Hai "risolto" l'eccezione NullReferenceException qui, ma ad un costo eccezionale! Ciò imporrà a tutti i chiamanti di Room.ClientId di controllare 0:

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

Si possono creare strani bug con altri sviluppatori (o te stesso!) dimenticandosi di controllare il valore di ritorno "ErrorCode" ad un certo punto nel tempo.
Gioca in sicurezza e fallisci velocemente. Permetti di lanciare la NullReferenceException e "aggraziatamente" catturarla da qualche parte più in alto nello stack delle chiamate invece di permettere al chiamante di rovinare ancora di più le cose ...

Su una nota diversa, se sei così desideroso di esporre Client e anche ClientId pensa a TellDontAsk . Se chiedi troppo agli oggetti invece di dire loro cosa vuoi ottenere, puoi finire per accoppiare tutto insieme, rendendo più difficile il cambiamento in seguito.

    
risposta data 24.08.2015 - 21:24
fonte
1

A partire da c # 6.0, che è ora disponibile, dovresti fare questo,

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

Aumentare una proprietà di Client in Room è una chiara violazione dell'incapsulamento e del principio di DRY.

Se devi accedere al Id di un Client di un Room che puoi fare,

var clientId = room.Client?.Id;

Nota l'uso del operatore condizionale Null , clientId sarà un long? , se Client è null , clientId sarà null , altrimenti clientId avrà il valore di Client.Id .

    
risposta data 25.08.2015 - 10:39
fonte

Leggi altre domande sui tag