Si dovrebbe creare un membro di classe privato condivisibile o mantenere variabile nell'ambito del metodo per passarlo come secondo argomento del metodo?

3

Recentemente ho dovuto refactoring alcuni codici legacy. Come nella maggior parte dei casi, ho dovuto dividere grandi parti del codice in funzioni più piccole, più pulite e leggibili. Ho finito con molte funzioni, che avevano più parametri strani. Fammi mostrare un esempio:

public void SendOrder(Order order, XmlOrder xmlOrder, Insider insider)
{
    string customerCode = CustomerServices
                 .Get(insider.CustomerId)
                 .Code;

    OutputData outputData = CreateOutputData(order, xmlOrder, customerCode);
    CreateReservations(order, customerCode, outputData);
    PlaceOrder(order, xmlOrder, outputData, customerCode);
}
...
private void CreateReservations(Order order, string customerCode, OutputData outputData)
{
    ...
    try
    {
        ReservationServices.AddReservation(reservation);
    }
    catch (BusinessException ex)
    {
        Logger.Log(ex);
        outpuData.Status = Statuses.BusnessError;
        throw;
    }
}

(Questo è solo un codice dimostrativo, non reale)

Il problema è che dovevo passare ad outputData ad altre funzioni solo per cambiare il suo stato se si verifica un'eccezione o passare customerCode a più funzioni. La classe è responsabile dell'invio di più messaggi non connessi a WebService, quindi, quando lo stavo creando, non avevo intenzione di inserire le variabili connesse a un determinato ordine come stato di una classe. È una buona pratica escludere tali variabili dalla funzione e renderle membri della classe? Ci sono delle linee guida per tali situazioni? Quali sono le tue pratiche e soluzioni per tali problemi?

    
posta helvy91 28.01.2018 - 13:16
fonte

2 risposte

3

Should one create shareable private class member or keep variable in method scope to pass it as a second method argument?

C'è una terza opzione, che è quella di indirizzare formalmente separazioni logicamente separate introducendo un'altra classe.

Class is responsible of sending multiple, not connected messages to WebService, so when I was creating it, I didn't plan to put variables connected to given order as a class state.

Mi sembra che tu abbia a che fare con diverse astrazioni qui.

A volte usiamo più parametri invece di creare un'astrazione formale. Ad esempio, abbiamo una coordinata x e una coordinata y , e li usiamo come parametri in più punti. Sono pensati per essere presi in coppia - sono quindi un'astrazione informale. Per creare un'astrazione formale, possiamo creare una classe Point che lega insieme x e y . Ora dove mai abbiamo passato le due variabili, passiamo la singola astrazione. Una singola astrazione è migliore delle coppie di variabili in quanto il client che utilizza queste ha meno cose di cui preoccuparsi, e Point è più sicuro di tipo e meno incline agli errori che i singoli valori di coordinate (che potrebbero essere int s, per esempio). Ad esempio, uno potrebbe scambiare accidentalmente x e y ; uno potrebbe usare x con il y sbagliato quando si ha a che fare con più punti.

In generale, sarebbe un errore includere i campi di un'astrazione logicamente diversa come membri di qualche altra astrazione. Mentre è difficile vedere dal codice nel tuo esempio, che cosa sta facendo la classe in questione, non sembra una buona casa solo per passare parametri a queste altre funzioni (metodi?).

Quando decidi se aggiungere campi a una classe esistente e creare creare un'altra classe, ecco un modo per pensarci. Se sono astrazioni logicamente separate, allora dovrebbero avere classi separate. Se sono una sola e stessa astrazione, quindi aggiungi i campi alla classe. Controlla la durata dei singoli campi. Se hanno tutti la stessa durata (e tutta la stessa durata dell'oggetto (le istanze della classe)), ciò è coerente con l'essere un'unica astrazione logica. Tuttavia, se alcuni dei campi sono non inizializzati dopo che gli altri sono stati correttamente inizializzati (ad esempio nel costruttore o in un metodo init) e gli altri campi sono solo inizializzati e validi durante certe sequenze operative, allora appartengono ad un'astrazione separata logicamente, che indica classi separate.

Nel tuo esempio, order e xmlorder possono formare un'astrazione separata dal punto di vista logico affinché tu possa ricevere come parametro nel primo metodo.

Inoltre, outputData potrebbe essere aggiornato (aggiungendo dei campi ad esso o creando un wrapper - usa l'analisi lifetime precedente per determinare) per catturare ( order e xmlorder ) e outputData e customerCode .

Quindi il tuo codice potrebbe apparire come questo:

public void SendOrder(OrderBundle orderBundle, Insider insider)
{
    string customerCode = CustomerServices
                 .Get(insider.CustomerId)
                 .Code;

    OutputDataBundle outputBundle = CreateOutputData(orderBundle, customerCode);
    outputBundle.CreateReservations();
    outputBunlde.PlaceOrder();
}

(Nonostante il tipo di qualità e i nomi delle variabili.)

    
risposta data 28.01.2018 - 17:04
fonte
0

I membri della classe non dovrebbero mai essere dati temporanei di archiviazione per alcune procedure, che risulterebbero rumorosi e confusi per chiunque tenti di comprendere la natura della classe. Naturalmente se si taglia a caso una lunga funzione solo per sostituirla con un numero di funzioni con un numero limitato di righe, si incontreranno problemi con scope variabile. Una variabile locale non può essere nell'ambito di più funzioni.

Quindi, prima di tagliare una funzione "perché è troppo lunga", prova a capire che cosa effettivamente fa logicamente e inizia da lì. Ogni passaggio può produrre un risultato e non deve sapere nulla di cui gli altri passaggi devono essere a conoscenza. Riconoscere e isolare quelle parti che sono indipendenti dalla logica principale. Quelli sono quelli per cui vuoi funzioni separate.

    
risposta data 28.01.2018 - 19:35
fonte