2010-08-11 9 views
5

Ho trovato il seguente pezzo di codice durante una revisione del codice.Restituzione di un nuovo oggetto rispetto alla modifica di uno passato come parametro

Il mio intuito mi sta dicendo che questo non sta seguendo l'OOP corretto.

Sto pensando che invece il metodo LoadObject dovrebbe restituire un nuovo oggetto SomeObject, invece di modificare quello passato in esso. Anche se non riesco a trovare una spiegazione adeguata del perché è meglio.

La mia soluzione è migliore? e se sì perché? in particolare quali principi o standard OOP sono infranti nell'esempio di codice fornito (se esiste)?

public void someMethod() 
    { 
     ... 
     var someObject = new SomeObject(); 
     LoadSomeObject(reader,someObject); 
    } 

    private void LoadSomeObject(SqlDataReader reader, SomeObject someObject) 
    { 
     someObject.Id = reader.GetGuid(0); 
    } 
+0

Sembra che tu possa anche cambiare "SqlDataReader'. –

risposta

1

Non sono un guru OO, quindi prendi questi con un pizzico di sale.

  1. oggetti devono gestirsi/definire il loro comportamento se stessi il più lontano possibile. La logica alla base di questo è ovvia se hai mai a che fare con accoppiamenti lenti. Potrei benissimo essere la migliore decisione di progettazione per spostare i dettagli di LoadSomeObject all'implementazione di SomeObject, ma è difficile discuterne per un esempio così generale.

  2. Lo stato mutevole è perfettamente valido in qualsiasi codice imperativo (incluso il codice OO), è una "caratteristica" fondamentale di questi paradigmi. OTOH, lo stato immutabile ha vantaggi indiscutibili (credo che abbiamo qualche domanda su questo argomento qui, altrimenti chiedi a qualsiasi sostenitore della FP), e avere oggetti immutabili non è specialmente non OO.

Modifica: è possibile passare il lettore al costruttore di SomeObject.

+0

Qualche ragione particolare è stata accettata rispetto alla risposta di russjudge con un numero di revisioni quattro volte superiore? – delnan

8

Non c'è niente di sbagliato nel modo in cui il codice viene scritto poiché si modifica solo una proprietà su someObject.

Tuttavia, sarebbe altrettanto corretto creare alcuni oggetti all'interno di LoadSomeObject e restituirli.

A questo punto, entrambe le scelte sono corrette.

0

In entrambi i casi è perfettamente accettabile, ma le considerazioni su ciò che si desidera fare con l'oggetto restituito entrano in gioco quando si decide quale è giusto per la propria situazione particolare. Immutabilità creando una nuova istanza in ogni operazione è come vengono gestite le stringhe in .NET.

Un metodo di copia dovrebbe ovviamente restituire una copia. Laddove un metodo che funziona su un oggetto singleton, ad esempio che è contenuto da un riferimento globale, non si addice alla restituzione di un nuovo oggetto in quanto le modifiche potrebbero andare perse.

0

Se LoadSomeObject restituisce un nuovo SomeObject, è possibile modificare il nome del metodo per evitare confusione. Forse a NewAndLoadSomeObject?

0

Sono d'accordo con la tua intuizione, che si sente un po 'fuori per la maggior parte dei casi. Sono d'accordo sul fatto che restituire qualcosa è meglio, poiché rende solo più chiaro che qualcosa viene modificato. Vedere un valore restituito in uso rende immediatamente chiaro cosa sta succedendo.

penso che questo si sente un po 'meglio, anche se (nella maggior parte dei casi comunque):

public void someMethod() 
{ 
    ... 
    var someObject = new SomeObject(); 
    someObject.Load(reader); 
} 

E poi, ovviamente, nella classe SomeObject, avreste

public void Load(SqlDataReader reader) 
{ 
    this.Id = reader.GetGuid(0); 
} 

L'idea è quella di favorire i metodi di istanza su quelli statici. Perché preoccuparsi di creare un oggetto e trasmettere i propri dati quando si può semplicemente far funzionare l'oggetto sui propri dati?

1

Non esiste un concetto OO universale, che è in conflitto con un codice del genere. Ma poi scoprirai che è difficile raccogliere e capire come manipolare le istanze di SomeObject se non seguirai alcuni principi di progettazione. modo Forse più semplice per antipasto è quello di separare due tipi principali di procedure:

  • funzionale - che è stato progettato per creare nuove istanze e non mutano altri oggetti.
  • Methodic - progettato per modificare lo stato dell'istanza host.

Quindi buona idea qui, se si vuole separare SomeObject manipolare logica è di creare tipo SomeObjectBuilder con

public void loadFromReader(SqlDataReader reader) 

metodo e

pubblico SomeObject getValue()

immobile