I costruttori dovrebbero mai essere usati solo per gli effetti collaterali?

4

Riepilogo: Perché è sbagliato progettare un costruttore solo per i suoi effetti collaterali e quindi utilizzare il costruttore senza assegnare mai il suo valore di ritorno a una variabile?

Sto lavorando a un progetto che prevede la modellazione di giochi di carte. L'API pubblica per il gioco utilizza uno schema dispari per istanziare oggetti Rank e Suit. Entrambi utilizzano un elenco statico per tenere traccia delle istanze. In un mazzo standard, ci saranno sempre solo quattro oggetti Suit e tredici Classifica. Quando costruisci una carta, i ranghi e le tute si ottengono con una getRank() statica e una getSuit() , che restituisce l'oggetto identificato dall'elenco statico. Rank e Suit hanno altri metodi statici come removeRank() e clear() .

Ciò che è strano è che non esiste un metodo statico add() per aggiungere un nuovo Rank o Suit alla lista statica. Invece, questo comportamento si verifica solo quando un costruttore Rank o Suit viene invocato tramite new . Poiché le istanze di Rank e Suit sono ottenute con lo statico getRank() , il valore restituito dai nuovi costruttori è completamente irrilevante.

Alcuni codice di esempio potrebbe assomigliare a questo

//set up the standard suits
Suit.clear(); //remove all Suit instances from the static list
new Suit("clubs", 0);  //add a new instance, throw away the returned value
new Suit("diamonds", 1);
new Suit("hearts", 2);
new Suit("spades", 3);

//create a card
Deck theDeck = new Deck()
for (int i = 0; i < 4; i++) {
    for (int j = 0; j < 13; j++) {
        Suit theSuit = Suit.getSuit(i);
        Rank theRank = Rank.getRank(j);
        theDeck.add(new Card(theSuit, theRank));
    }
}

Non ho mai visto codice che non assegni l'oggetto restituito da un costruttore. Non pensavo nemmeno che fosse possibile farlo in Java fino a quando non l'ho testato. Tuttavia, quando è stato sollevato il problema che mancava il metodo statico addSuit() , la risposta era

"Objects of type Suit and Rank are created via constructors. The intent is to mimic the behavior of the java.lang.Enum class in which objects are created via constructors and the class maintains the collection of such objects. Current behavior is designed as intended."

L'uso di costruttori in questo modo mi fa davvero male, e voglio riaprire il problema, ma non riesco a trovare molto materiale a supporto (linee guida per la codifica, anti-pattern, altra documentazione). Si tratta in realtà di uno schema di codifica valido che non ho mai incontrato prima? In caso contrario, quale è un buon argomento che posso fare per far cambiare l'API?

Questo è un articolo che parla di ciò che i costruttori dovrebbero fare:

We can agree that a constructor always needs to initialize new instances, and we might even agree by now that a constructor always needs initialize instances completely.

Le modifiche: Aggiunte informazioni aggiuntive sulla risposta del progettista al problema. Aggiunti alcuni articoli che parlano di ciò che i costruttori dovrebbero e non dovrebbero fare.

    
posta BrianHVB 04.03.2017 - 05:30
fonte

5 risposte

11

Certo, tecnicamente questo può funzionare, ma viola il cosiddetto Principio di minimo stupore , perché la convenzione tipica per come vengono utilizzati i costruttori e per cosa sono buoni è piuttosto diverso. Inoltre, viola il SRP , poiché ora i costruttori fanno due cose invece di quello che di solito dovrebbero fare - costruire l'oggetto e che lo aggiungono a un elenco statico.

In breve, questo è un esempio per il codice di fantasia, ma non per un buon codice.

    
risposta data 04.03.2017 - 09:32
fonte
6

Non vedo ragioni sensate per questo "modello". Come dici tu, la funzionalità desiderata sarebbe meglio implementata da una funzione statica; mentre è probabilmente / probabilmente vero che usare un costruttore per fare questo non produce alcun problema funzionale, infrange il Principio di Least Surprise: come te, guardo quel codice e penso "Eh !? Cosa sta succedendo qui allora? ". Non vedo alcun vantaggio in in questo modo rispetto alla funzione statica.

Facendo un passo indietro, la natura globale delle tute è anche un odore di codice: cosa succede se vuoi modellare sia il bridge che i (dire) tarocchi, con i suoi semi diversi nello stesso programma? Un design migliore implicherebbe un SuitCollection o simile.

Che cosa puoi fare a riguardo? Questo dipende molto dal contesto. Se questo è un progetto commerciale, parla con i tuoi colleghi per vedere se c'è una buona ragione per questa decisione, e se non vai vai a parlare con chiunque abbia questa risposta di persona e prova a discutere le cose. Se si tratta di un progetto open source o simile, il mio consiglio personale sarebbe "scappare": un codice di scarsa qualità combinato a una scarsa comunicazione non è destinato a un progetto divertente.

    
risposta data 04.03.2017 - 08:14
fonte
2

Prima di tutto, diciamo che uno degli obiettivi principali della progettazione del software è la manutenibilità, assicurandoti che il codice sia facile da capire e modificare.

Quindi, diciamo che il costruttore è un tipo speciale di metodo che inizializza un oggetto appena creato in uno stato valido.

Alla luce di questi due presupposti, è sicuro dire che non è valido avere un costruttore di effetti collaterali in una progettazione corretta. Costruttore per sua natura non dovrebbe avere alcun effetto oltre all'oggetto appena creato e agli oggetti che racchiude.

Dato questo codice:

public void MyMethod()
{
    DoSomething();
    new MyClass();
    DoSomething();
}

Lo troveresti facile da mantenere? Puoi riordinare MyClass() e DoSomething() ? Non lo sai finché non visiti il costruttore di MyClass e vedi cosa fa, giusto?

In questo modo, ti ritroverai presto a dover comprendere l'intero programma per modificarne piccole parti. Questo è l'esatto contrario del buon design.

Inoltre, avere un costruttore di effetti collaterali di solito implica che c'è qualcosa di statico in corso in background, che è di nuovo una bandiera rossa quando si tratta di test e di una corretta progettazione. Sebbene apparentemente economico e gestibile, il contesto statico cresce molto velocemente e ostacola le future modifiche del programma.

Ovviamente, la programmazione è un lavoro di compromesso e se la presenza di una raccolta statica di tipi di carte otterrebbe grandi miglioramenti nel time-to-market (ho forti dubbi a riguardo), sarebbe ancora molto meglio vedere l'intento espresso tramite metodi statici e inizializzazione una tantum:

public void ApplicationSetup()
{
    Suit.Initialize("clubs", "diamonds", "hearts", "spades"); // throws if called more than once
}

Tuttavia, anche in questo modo, si può finire per chiamare parti del programma che richiedono Suit da inizializzare prima che l'inizializzazione avvenga effettivamente. Il modo migliore per esprimere "questa classe / metodo richiede Suit da inizializzare" consiste nel prendere Suit come parametro. Che è contrario all'idea di Suit statica.

    
risposta data 04.03.2017 - 09:19
fonte
1

No! È un brutto codice! È un modello confuso che non fornisce alcun vantaggio rispetto a farlo in modo semplice.

Il costruttore è usato solo come un metodo statico elaborato. Chiamarlo come Suit.addSuit("spades", 3); avrebbe lo stesso effetto senza confondere e restituire un'istanza inutile.

Il confronto con Enum non giustifica lo strano uso dei costruttori - le enumerazioni non usano questo schema. Con l'enumerazione, i costruttori vengono utilizzati per creare valori del tipo (ad esempio come alternativa a Suit.getSuit() ), non per definire le opzioni possibili, che avvengono tramite una sintassi dedicata.

Inoltre, l'uso di Suit.Clear() indica che questo non è affatto equivalente a Enums. Il punto di enumerazione è che i valori disponibili sono definiti e corretti al momento della compilazione e quindi possono essere controllati staticamente. Il metodo Clear() indica che le opzioni variano a seconda del tempo di esecuzione. Se hai diversi set di semi in fase di esecuzione, non dovrebbe essere definito come una raccolta statica.

    
risposta data 04.03.2017 - 10:17
fonte
1

Sono d'accordo con le altre risposte sul fatto che questo modello è scadente e viola vari principi di ingegneria del software. A quanto pare i tuoi colleghi non si preoccupano molto di loro. Quello di cui hai bisogno sono esempi di casi d'uso in cui questo modello fallisce o è scomodo da usare. Ecco un paio:

  1. Il gioco è Bridge. Dopo l'offerta, a volte è necessario cambiare il nome di uno dei quattro semi esistenti in "briscola". Prova a scrivere quel codice. Prevedo che sarà icky, buggy e potrebbe anche rompere il codice esistente.
  2. Il gioco è Hearts. Rinominare la regina di cuori in "The Lady". (Versione PG).
  3. Il tuo gioco è un grande successo in Cina e in Armenia. Aggiungi un menu per la lingua che possono scegliere in qualsiasi momento durante il gioco. Devi ottenere questa versione sul mercato prima che i tuoi concorrenti lo facciano! Oh sì, è una partita multiplayer e la grande partita tra Armenia e Shanghai sta arrivando, con un pubblico televisivo via cavo negli Stati Uniti.
risposta data 04.03.2017 - 17:25
fonte

Leggi altre domande sui tag