Passa la variabile membro come parametro del metodo

31

In un progetto, ho trovato un codice come questo:

class SomeClass
{
    private SomeType _someField;

    public SomeType SomeField
    {
        get { return _someField; }
        set { _someField = value; }
    }

    protected virtual void SomeMethod(/*...., */SomeType someVar)
    {
    }

    private void SomeAnotherMethod()
    {
        //.............
        SomeMethod(_someField);
        //.............
    }

};

Come faccio a convincere i miei compagni di squadra che si tratta di codice errato?

Credo che questa sia una complicazione inutile. Perché passare una variabile membro come parametro del metodo se si ha già accesso ad essa? Questa è anche una violazione dell'incapsulamento.

Vedi altri problemi con questo codice?

    
posta tika 29.04.2012 - 17:39
fonte

10 risposte

2

Penso che questo sia un argomento valido, ma il motivo per cui si ottengono risposte miste dipende dal modo in cui è stata formata la domanda. Personalmente, ho avuto le stesse esperienze con la mia squadra dove passare i membri come argomenti era inutile e contorto il codice. Avremmo una classe che funziona con un insieme di membri, ma alcune funzioni accedono direttamente ai membri e altre funzioni modificherebbero gli stessi membri attraverso parametri (ad esempio, usano nomi completamente diversi) e non c'era assolutamente alcuna ragione tecnica per farlo. Per ragioni tecniche, intendo un esempio fornito da Kate.

Suggerirei di fare un passo indietro e invece di concentrarsi esattamente sul passaggio dei membri come parametri, avviare discussioni con il tuo team sulla chiarezza e leggibilità. O più formalmente, o semplicemente nei corridoi, discutete su cosa rende più facile leggere alcuni segmenti di codice e altri segmenti di codice più difficili. Quindi identifica le misure di qualità o gli attributi di codice pulito che come squadra vorresti ottenere. Dopotutto, anche quando lavoriamo su progetti di campo verde, dedichiamo più del 90% del tempo a leggere e non appena il codice viene scritto (diciamo 10-15 minuti più tardi) va in manutenzione dove la leggibilità è ancora più importante.

Quindi, per il tuo esempio specifico qui, l'argomento che userei è che meno codice è sempre più facile da leggere di più codice. Una funzione che ha 3 parametri è più difficile da elaborare per il cervello rispetto a una funzione che non ha nessuno o 1 parametro. Se c'è un altro nome di variabile, il cervello deve tenere traccia di un'altra cosa mentre legge il codice. Quindi, per ricordare "int m_value" e poi "int localValue" e ricordare che uno in realtà significa che l'altro è sempre più costoso per il tuo cervello, quindi funziona semplicemente con "m_value".

Per ulteriori munizioni e idee, ti consiglio di prendere una copia del codice pulito     

risposta data 29.04.2012 - 20:13
fonte
28

Posso pensare a una giustificazione per il passaggio di un campo membro come parametro in un metodo (privato): rende esplicito da cosa dipende il tuo metodo.

Come dici tu, tutti i campi membri sono parametri impliciti del tuo metodo, perché l'intero oggetto lo è. Tuttavia, l'oggetto completo è davvero necessario per calcolare il risultato? Se SomeMethod è un metodo interno che dipende solo da _someField , non è più pulito rendere esplicita questa dipendenza? In effetti, rendere esplicita questa dipendenza può anche suggerire che puoi effettivamente refactoring questo pezzo di codice dalla tua classe! (Nota suppongo che non stiamo parlando di getter o setter qui, ma codice che calcola effettivamente qualcosa)

Non farei lo stesso argomento per i metodi pubblici, perché il chiamante non sa né importa quale parte dell'oggetto sia rilevante per calcolare il risultato ...

    
risposta data 29.04.2012 - 18:17
fonte
24

Vedo un motivo valido per il passaggio di variabili membro come argomenti della funzione ai metodi private - purezza della funzione. Le variabili membro sono effettivamente uno stato globale dal punto di vista, per di più, uno stato globale mutabile se detto membro cambia durante l'esecuzione del metodo. Sostituendo i riferimenti delle variabili membro con i parametri del metodo, possiamo effettivamente rendere pura una funzione. Le funzioni pure non dipendono da uno stato esterno e non hanno effetti collaterali, restituendo sempre gli stessi risultati in base allo stesso set di parametri di input, semplificando così i test e la manutenzione futura.

Certo, non è né facile né pratico avere tutti i tuoi metodi come metodi puri in un linguaggio OOP. Ma credo che guadagni molto in termini di chiarezza del codice avendo i metodi che gestiscono la logica complessa pura e utilizzando variabili immutabili, pur mantenendo la gestione dello stato "globale" impura separata all'interno di metodi dedicati.

Passare le variabili membro a una funzione pubblica dello stesso oggetto quando si chiama la funzione esternamente sarebbe comunque un odore di codice importante a mio parere.

    
risposta data 30.04.2012 - 00:27
fonte
7

Se la funzione viene chiamata varie volte, a volte passando quella variabile membro e talvolta passando qualcos'altro, allora è OK. Ad esempio, non lo considererei affatto negativo:

if ( CalculateCharges(newStartDate) > CalculateCharges(m_StartDate) )
{
     //handle increase in charges
}

dove newStartDate è una variabile locale e m_StartDate è una variabile membro.

Tuttavia se la funzione viene sempre chiamata solo con la variabile membro passata ad essa, è strano. Le funzioni membro lavorano sempre su variabili membro. Potrebbero farlo (a seconda della lingua in cui stai lavorando) per ottenere una copia della variabile membro - se è così e non puoi vederlo, il codice potrebbe essere più bello se rendesse esplicito l'intero processo.

    
risposta data 29.04.2012 - 19:17
fonte
1

I framework GUI di solito hanno una sorta di classe 'View' che rappresenta le cose disegnate sullo schermo, e quella classe di solito fornisce un metodo come invalidateRect(Rect r) per contrassegnare parte della sua area di disegno come se fosse necessario ridisegnarlo. I client potrebbero chiamare questo metodo per richiedere un aggiornamento a parte della vista. Ma una vista potrebbe anche chiamare il proprio metodo, come:

invalidateRect(m_frame);

per ridisegnare l'intera area. Ad esempio, potrebbe farlo quando viene prima aggiunto alla gerarchia della vista.

Non c'è niente di sbagliato nel fare ciò: il frame della vista è un rettangolo valido e la vista stessa sa che vuole ridisegnarsi. La classe View può fornire un metodo separato che non accetta parametri e utilizza invece il frame della vista:

invalidateFrame();

Ma perché aggiungere un metodo speciale solo per questo quando puoi usare il più generale invalidateRect() ? Oppure, se hai scelto di fornire invalidateFrame() , molto probabilmente lo implementerai in termini di invalidateRect() più generico:

View::invalidateFrame(void)
{
    invalidateRect(m_frame)
}

Why pass variable as method parameter, if you already has access to it?

dovresti passare le variabili di istanza come parametri al tuo metodo se il metodo non opera in modo specifico su quella variabile di istanza. Nell'esempio sopra, la cornice della vista è solo un altro rettangolo per quanto riguarda il metodo invalidateRect() .

    
risposta data 29.04.2012 - 20:25
fonte
1

Ciò che nessuno ha toccato è che SomeMethod è protetto virtuale. Significa che una classe derivata può usarlo E ri-implementare la sua funzionalità. Una classe derivata non avrebbe accesso alla variabile privata e quindi non potrebbe fornire un'implementazione personalizzata di SomeMethod che dipende dalla variabile privata. Invece di prendere la dipendenza dalla variabile privata, la dichiarazione richiede al chiamante di passarlo.

    
risposta data 30.04.2012 - 02:24
fonte
1

Questo ha senso se il metodo è un metodo di utilità. Supponiamo ad esempio che tu debba ricavare un nome breve univoco da diverse stringhe di testo libero.

Non vorresti codificare un'implementazione separata per ogni stringa, ma passare la stringa a un metodo comune ha senso.

Tuttavia, se il metodo funziona sempre su un singolo membro, sembra un po 'stupido passarlo come parametro.

    
risposta data 30.04.2012 - 09:59
fonte
1

La ragione principale per cui una variabile membro di una classe è consentire di impostarla in un unico posto e avere il suo valore disponibile per tutti gli altri metodi della classe. In generale, quindi, ti aspetteresti che non sia necessario passare la variabile membro in un metodo della classe.

Posso tuttavia pensare a un paio di motivi per cui potresti voler passare la variabile membro in un altro metodo della classe. Il primo è se è necessario garantire che il valore della variabile membro debba essere usato invariato quando usato con il metodo chiamato, anche se quel metodo dovrà cambiare il valore della variabile membro effettivo in qualche punto del processo. Il secondo motivo è legato al primo in quanto si potrebbe desiderare di garantire l'immutabilità di un valore nell'ambito di una catena di metodi, ad esempio quando si implementa una sintassi fluente.

Tenendo presente tutto ciò, non direi che il codice sia "cattivo" in quanto tale se passi una variabile membro a uno dei suoi metodi di classe. Vorrei comunque suggerire che non è generalmente l'ideale, in quanto potrebbe incoraggiare un sacco di duplicazione del codice in cui il parametro è assegnato a una variabile locale, ad esempio, e i parametri aggiuntivi aggiungono "rumore" nel codice in cui non è necessario. Se sei un fan del libro Clean Code, dovresti sapere che si dovrebbe mantenere il numero di parametri del metodo al minimo indispensabile e solo se non c'è un altro modo più ragionevole per il metodo di accedere al parametro .

    
risposta data 03.05.2012 - 06:05
fonte
1

Alcune cose che vengono da me quando pensi a questo:

  1. In generale, le firme dei metodi con meno parametri sono più facili da capire; un grande motivo per cui sono stati inventati i metodi era l'eliminazione di lunghi elenchi di parametri sposandoli ai dati su cui operano.
  2. Il fatto che la firma del metodo dipenda dalla variabile membro renderà più difficile modificare tali variabili in futuro, poiché non solo è necessario cambiare all'interno del metodo, ma ovunque viene chiamato anche il metodo. E poiché SomeMethod nel tuo esempio è protetto, anche le sottoclassi dovranno cambiare.
  3. I metodi (pubblici o privati) che non dipendono dagli interni della classe non hanno bisogno di essere su quella classe. Potrebbero essere scomposti in metodi di utilità e essere altrettanto felici. Non hanno quasi nessuna parte in questa classe. Se nessun altro metodo dipende da quella variabile dopo aver spostato il / i metodo / i, allora anche quella variabile dovrebbe andare! Molto probabilmente la variabile dovrebbe essere sul proprio oggetto con quel metodo o metodi che operano su di esso diventando pubblico e composto dalla classe genitore.
  4. Trasmettere dati a varie funzioni come la tua classe è un programma procedurale con variabili globali che sfidano semplicemente la progettazione OO. Questo non è l'intento delle variabili membro e delle funzioni membro (vedi sopra), e sembra che la tua classe non sia molto coesa. Penso che sia un odore di codice che suggerisce un modo migliore per raggruppare i dati e i metodi.
risposta data 20.08.2013 - 07:15
fonte
0

Manca se il parametro ("argomento") è di riferimento o di sola lettura.

class SomeClass
{
    protected SomeType _someField;
    public SomeType SomeField
    {
        get { return _someField; }

        set {
          if (doSomeValidation(value))
          {
            _someField = value;
          }
        }
    }

    protected virtual void ModifyMethod(/*...., */ ref SomeType someVar)
    { 
      // ...
    }    

    protected virtual void ReadMethod(/*...., */ SomeType someVar)
    { 
      // ...
    }

    private void SomeAnotherMethod()
    {
        //.............

        // not recommended, but, may be required in some cases
        ModifyMethod(ref this._someField);

        //.............

        // recommended, but, verbose
        SomeType SomeVar = this.someField;
        ModifyMethod(ref SomeVar);
        this.someField = SomeVar;

        //.............

        ReadMethod(this.someField);
        //.............
    }

};

Molti sviluppatori, di solito assegnano direttamente i campi interni di una variabile, nei metodi del costruttore. Ci sono alcune eccezioni.

Ricorda che i "setter" possono avere metodi aggiuntivi, non solo assegnamenti e, a volte, persino metodi virtuali, o metodi virtuali.

Nota: suggerisco di mantenere i campi interni delle proprietà come "protetti".

    
risposta data 29.04.2012 - 23:31
fonte

Leggi altre domande sui tag