È una pratica scorretta avere una classe implementare una classe astratta senza aggiungere nuovi campi / membri / funzionalità?

5

Sto scrivendo un wrapper per un'API REST e mi sono imbattuto in qualcosa che non avevo mai dovuto chiedermi prima.

Questa API è per le transazioni E-Commerce, ha endpoint SALE e RETURN (e alcuni altri endpoint che non sono fondamentali per questa discussione).

Le richieste SALE e RETURN sono quasi identiche, con l'eccezione che la richiesta SALE ha una proprietà aggiuntiva.

public class ReturnRequest
{
    [JsonProperty("cashier_id")]
    public string CashierId { get; set; }

    [JsonProperty("amount")]
    public decimal Amount { get; set; }

    [JsonProperty("pos_id")]
    public CustomClass PosId { get; set; } 
}

public class SaleRequest
{
    [JsonProperty("cashier_id")]
    public string CashierId { get; set; }

    [JsonProperty("amount")]
    public decimal Amount { get; set; }

    [JsonProperty("pos_id")]
    public CustomClass PosId { get; set; } 

    [JsonProperty("zip_code")]
    public string ZipCode { get; set; }
}

Originariamente avevo appena ricevuto la richiesta di vendita POCO che eredita la richiesta di ritorno POCO:

public class ReturnRequest
{
    [JsonProperty("cashier_id")]
    public string CashierId { get; set; }

    [JsonProperty("amount")]
    public decimal Amount { get; set; }

    [JsonProperty("pos_id")]
    public CustomClass PosId { get; set; }  
}

public class SaleRequest : ReturnRequest
{
    [JsonProperty("zip_code")]
    public string ZipCode { get; set; }
}

Ma a me non sembra molto intuitivo o chiaro (SALE e RITORNO sono cose diverse, perché ne erediterebbe un altro?)

Ho quindi deciso di inserire le proprietà comuni in una classe astratta di base:

public abstract class BaseRequest
{
    [JsonProperty("cashier_id")]
    public string CashierId { get; set; }

    [JsonProperty("amount")]
    public decimal Amount { get; set; }

    [JsonProperty("pos_id")]
    public CustomClass PosId { get; set; }  
}

public class ReturnRequest : BaseRequest
{

}

public class SaleRequest : BaseRequest
{
    [JsonProperty("zip_code")]
    public string ZipCode { get; set; }
}

Ma poi questo mi lascia con una classe che è essenzialmente la stessa cosa di BaseRequest.

All'inizio ho giustificato questo perché volevo che le classi fossero semplici e dettagliate. Crea una chiara differenza tra le classi e posso modificare SALE o RETURN senza preoccuparmi dell'altro (o anche delle classi base).

Tuttavia ora mi chiedo se sto pensando a questo torto, che sia essenzialmente una classe vuota una cattiva idea?

EDIT: ho modificato i nomi delle proprietà e digitato un po 'per rappresentare più da vicino alcuni tipi di proprietà.

    
posta maccettura 27.01.2017 - 00:58
fonte

3 risposte

7

Il mio esempio preferito di ereditare e aggiungere niente di nuovo è questo:

public class LoginException : System.Exception {}

Perché? Perché ho detto tutto quello che devo dire con quel nome di classe meravigliosamente descrittivo. Certo che se voglio aggiungere un messaggio dinamico ho bisogno di aggiungere un costruttore. Ma se non lo faccio, perché dovrei?

Il tuo esempio manca di questa chiarezza perché i problemi semantici sono nascosti con nomi come Prop1 . Mi piace sostituire l'ereditarietà con la composizione e la delega, ma in questo caso, quando lo faccio, finisco per osservare questo:

SaleRequest sr = new SaleRequest(new ReturnRequest());

Ora devo chiedere, questo ha senso semantico? Non posso richiedere una vendita senza richiedere un reso?

Certo, meccanicamente ha senso perché i 'Puntelli' sono 'corretti' ma quelli sono solo tipi di dati. Avrebbe ancora senso se avessero nomi significativi?

Il tuo tentativo di risolvere questo problema è simile a questo:

SaleRequest sr = new SaleRequest(new BaseRequest());

Che di nuovo è semanticamente anemico. Base? Questo non mi dice cosa appartiene a questa classe e cosa non ci appartiene. Non è un buon nome.

Se BaseRequest aveva un nome che rendeva chiaro che Prop1 e Prop2 (qualunque essi siano) fanno parte di esso e BaseRequest ha un nome per qualcosa che è chiaramente parte di, o tutto, il altre due richieste quindi mi andrebbe bene con questo.

Allo stato attuale, non so cosa si suppone stia succedendo quando guardo questo codice. Non è una buona cosa.

Non ho alcun problema con le classi vuote. Ma per favore, dai loro dei bei nomi. È davvero l'unica cosa che hanno da offrire.

    
risposta data 27.01.2017 - 03:34
fonte
2

Sì, è assolutamente ragionevole avere sottoclassi "vuote" di una classe base. Una situazione diversa, ma in qualche modo analoga, sta avvolgendo un tipo per imporre una distinzione di tipo anche quando non ci sono differenze nella rappresentazione. Ad esempio:

struct Seconds {
    public Seconds(int v) { Value = v; }
    public int Value { get; }
}

struct Meters {
    public Meters(int v) { Value = v; }
    public int Value { get; }
}

Potrebbero non esserci motivi per pensare che questi saranno mai qualcosa di diverso da int s ma è comunque utile avvolgerli in modo che non vengano confusi.

Detto questo, per il tuo caso particolare, non è chiaro che BaseRequest corrisponda a una nozione significativa. Se aggiungi un'opzione LAYAWAY, BaseRequest sarà ancora una superclasse significativa? Se la risposta è "no", probabilmente tratterei SaleRequest e ReturnRequest come classi completamente separate. (Forse c'è un'interfaccia che cattura le caratteristiche generali di questi, ma potrebbe non essere BaseRequest .) Potresti usare BaseRequest puramente internamente come dettaglio di implementazione se aggiunge abbastanza convenienza, ma probabilmente non ne vale la pena questo caso, e non è una grande idea concettualmente (rende il tuo codice meno ovvio).

    
risposta data 27.01.2017 - 02:09
fonte
0

In questo caso, essendo due endpoint diversi, non condividerei il modello restituito, ma ognuno di essi ha il suo. Anche se ciò significa, in prima battuta, un codice "duplicato". Possono essere uguali ora, ma cambieranno / potrebbero cambiare per diversi motivi.

Non è un'astrazione comune, finché non hai un codice che si aspetta e lavora direttamente su "BaseRequest". Considera la loro somiglianza in modo casuale.

    
risposta data 27.01.2017 - 02:02
fonte

Leggi altre domande sui tag