Costruttore di refactoring che ha troppi parametri [duplicato]

Hai ragione, 8 parametri, tutte stringhe, rendono il costruttore un candidato per una revisione del codice.

Considera alcuni dei punti seguenti.

Attributi essenziali per primi

Guarda il modello del messaggio e indovina quali attributi sono essenziali per inizializzare un'istanza valida. Ridurre il numero di argomenti all'essenziale. Aggiungi setter per il resto.

Se tutti e 8 gli attributi sono richiesti e di sola lettura, non c'è molto da fare qui.

Encapsulation

Considera di incapsulare i parametri correlati. Ad esempio, a, b e c potrebbero andare insieme in una nuova classe. Ridurrà solo il numero di argomenti, ma non lo renderà più semplice necessariamente. Tuttavia, potrebbe contribuire ad esprimere meglio il modello e il riutilizzo del codice.

Utilizza motivi creativi

Invece di inizializzare i messaggi direttamente, fallo da fabbriche o costruttori .

Array

Se nessuno dei precedenti funziona per te, prova con un array di parametri. Data la mancanza di nomi param significativi, è probabilmente la soluzione più semplice.

Inheritance

Con il tempo ti renderai conto che i messaggi sono eccellenti candidati per l'ereditarietà. Segmentare i messaggi per attributo genera un piccolo sovraccarico perché ti troverai spesso a chiedere il tipo if(message.getType() == ...) .

Ecco una risposta eretica: Non vedo nulla di intrinsecamente sbagliato con molti parametri.

Avere una funzione che richiede molti parametri è spesso un obiettivo per il refactoring, ma solo perché abbiamo rifattorizzato qualcosa in una porzione di codice più fredda non significa che dovremmo averlo fatto.

L'uso di un array o di un array args invece di 8 parametri separati significa che avrete bisogno di una logica extra per far rispettare il numero corretto di parametri passati. Usare un modello di build è tutto ok, eccetto che se vengo indietro per mantenere il codice e avere un modello di builder per ogni singola stupida struttura di dati, è probabile che mi perda nelle sue linee di codice di bajillion. Se hai davvero bisogno di tutti e 8 gli argomenti, non puoi spostarne alcuni solo per i contenuti, perché potresti avere un'istanza della classe senza tutti i dati necessari.

So per esperienza personale che se sto ereditando un codice base, vorrei che il codice sia impostato in uno stile noioso, noioso, ovvio (come i tuoi 8 argomenti di costruzione, ecc. ecc.) invece che con qualche nuovo framework / pattern DI fresco / complesso che non abbia mai usato prima.

Se la tua classe sopra è una struttura dati semplice (auspicabilmente immutabile) che ha veramente bisogno di 8 argomenti di stringa per essere valida, allora io dico che avere 8 parametri è un modo perfettamente perfetto per farlo.

Lavorerò partendo dal presupposto che vuoi che questa classe sia immutabile.

Se si desidera l'immutabilità "vera", è necessario attenersi agli argomenti del costruttore, poiché il costruttore è l'unica parte di codice che può impostare un campo readonly .

Tuttavia, puoi anche solo implementare getter pubblici e setter privati. A tutti gli effetti, ciò rende anche l'oggetto immutabile. Avresti impostato i campi privati in un metodo factory factory (ish).

In questo esempio presumo che i dati RFID arrivino alla tua applicazione in un flusso, ma potresti facilmente modificarlo per lavorare con un array di byte, una serie di strutture, XML, qualunque cosa.

class RFIDMessage
{
    private string _a;
    private string _b;
    private string _c;
    //etc

    private RFIDMessage() 
    {
        //Declare default constructor private so nobody else can instantiate an RFIDMessage on their own
    }

    static public RFIDMessage FromStream(Stream stream)
    {
        var reader = new RFIDStreamReader(stream);
        var newObject = new RFIDMessage();
        newObject._a = reader.ReadA();
        newObject._b = reader.ReadB();
       //etc.

       return newObject;
    }
}

void SampleUsage()
{
    var rfidStream = RFIDAPI.GetStream(); //Or whatever
    RFIDMessage message = RFIDMessage.FromStream(rfidStream);
}

Non conosco il nome di questo modello, ma è comune nel CLR .NET, ad es. FromBase64String .

Supponiamo che tu abbia bisogno di tutti questi bit di dati in un'istanza ed eviti di mettere in discussione il design della classe. Forse la classe potrebbe avere meno membri e la quantità complessiva di codice potrebbe essere più piccola, se si creava un'altra classe di livello inferiore che raccoglieva un sottoinsieme di questi membri. Questa non è la domanda.

Se leggi il framework .net le linee guida di progettazione relative ai costruttori , tu si può vedere che sono preferiti numeri minori di parametri, sia perché i valori dei membri predefiniti possono ridurre il codice, sia perché è difficile passare molti parametri senza confondere l'ordine. L'alternativa è usare le proprietà per impostare l'istanza prima dell'uso. Impostare le proprietà che richiedono valori non predefiniti e lasciare gli altri così come sono. Questo può renderlo più facile da usare.

Poiché i campi di sola lettura devono essere preferiti su campi mutabili, quando i campi mutabili non sono necessari, c'è un compromesso. È possibile inizializzare solo i campi di sola lettura tramite costruttori. Non hai usato nessun campo di sola lettura, ma potresti voler riconsiderare questa decisione. È una debolezza dell'ambiente che ti costringe a prendere una decisione.

Anche se penso che il tuo codice potrebbe essere OK a seconda della situazione attuale, mi piacerebbe mostrare un'altra possibile soluzione: "sintassi fluente". L'idea alla base della sintassi fluente consiste nel fornire metodi per modificare lo stato dell'oggetto che restituisce l'oggetto modificato. Per es.

public RFIDMessage SetA(string a)
{
    this._a = a;
    return this;
}

Permette il codice come

RFIDMessage message = new RFIDMessage().SetA("Some A").SetC("Some C").SetH("Some H");

Si noti che ora l'istanza può essere modificata dopo la creazione, che potrebbe essere desiderata o un problema in base alle proprie esigenze. Se si tratta di un problema, è possibile creare una "interfaccia di sola lettura" che la classe implementa (e quei metodi SetA citati sopra non fanno parte dell'interfaccia) in modo tale che i consumatori dell'istanza che sanno che solo l'interfaccia di sola lettura non possa cambialo.