Le classi astratte vuote sono una cattiva pratica e perché?

Question

Abbiamo diverse classi astratte vuote nel nostro codebase. Lo trovo brutto. Ma oltre a questa ragione molto stupida (la bruttezza), dovrei rifattarla (in un'interfaccia vuota, ad esempio)?

In caso contrario, il codice è robusto e ben testato. Quindi, se è solo per una ragione "estetica", passerò e lascerò le classi astratte vuote.

Cosa ne pensi?

EDIT:

1) Con il termine "classe astratta vuoto", voglio dire qualcosa come:

public abstract class EmptyAbstractClass {} 

2) La ragione per il "vuoto": Sospensione. Non ho affatto padronanza di questo quadro di persistenza. Capisco solo che un'interfaccia non può essere mappata su una tabella, e per questo motivo tecnico, una classe è stata preferita a un'interfaccia.

Answer 1

Suoni come questo è il risultato della creazione di una gerarchia di oggetti che ha finito per non avere alcuna funzionalità comune al suo massimo livello. Sospetto che le sottoclassi immediate siano esse stesse astratte o almeno abbiano sottoclassi proprie. L'altra probabilità è che il tuo codice abbia molte istanze di funzioni sparse per tutto il suo codice.

Il livello più in alto vuoto non è un grosso problema in sé e per sé, ma vorrei verificare che non esiste effettivamente alcuna funzionalità comune. Supponendo che esista, vorrei esaminare le caratteristiche comuni nelle classi parent. Sicuramente cercherò di dare un'occhiata a instanceof e penserò seriamente al refactoring (si veda Refactoring to Patterns (Kerievsky) per gli esempi).

Answer 2

Le interfacce sono preferibili in questo caso perché rendono il codice più robusto per la manutenzione. Cioè, puoi estendere solo una singola classe ma puoi implementare molte interfacce.

Se al momento non c'è assolutamente alcun effetto diretto, non lo toccherò. Se si verifica l'evento di manutenzione che richiede di apportare una modifica, la riattribuisco perché sono già nel codice.

In altre parole se non è rotto non aggiustarlo.

Answer 3

La domanda da porsi è: "Cosa voglio fare con questo codice che non posso fare, o trovare difficile da fare, perché queste classi astratte vuote ci sono?" Se la risposta è "nulla", dovresti lasciarli soli. Se la risposta è "qualcosa", potrebbe valere la pena rimuoverli, ma se puoi, parla con le persone che li hanno creati per primi, solo per assicurarsi che non vi sia uno scopo sottile per loro. Ad esempio, forse il tuo codice usa la riflessione per trovare tutte le istanze di un particolare ABC e fare cose speciali per loro, nel qual caso il tuo refactoring potrebbe rompere il codice in modi sottili.

Answer 4

Poniti questa domanda: se, a seguito del tuo refactoring, qualcosa si rompe in produzione e il tuo continuo impiego dipende da quanto bene giustifichi la tua decisione di dedicare del tempo a sistemare qualcosa che non è stato effettivamente rotto, che cosa dici ?

"Era brutto ed esteticamente offensivo per me" non è una risposta su cui mi piacerebbe puntare il mio lavoro.

In questa fase, dico di andare sul sicuro e di vivere con il brutto.

Answer 5

Non è necessariamente più brutto dell'alternativa, che può essere codice ripetuto.

In un mondo ideale sarebbe possibile modellare utilizzando solo le interfacce. ad esempio: Veicoli -> Auto -> Pontiac.

Ma ci può essere una logica che è la stessa per tutti i veicoli, quindi un'interfaccia non è appropriata. E tu non hai una logica specifica per Cars. Ma tu vuoi un'astrazione Car perché il tuo TrafficLightController vuole distinguere tra Cars e Bicycles.

In tal caso è necessario creare e astrarre l'automobile.

Oppure è possibile creare un'interfaccia Veicolo, VeicoloImpl implementa Veicolo, un'interfaccia Auto estende Veicolo, un'interfaccia Pontiac implementa Auto, e un PontiacImpl implementa Pontiac estende VeicoloImpl. Personalmente non mi piace una gerarchia parallela di interfacce al fine di prevenire una classe astratta vuota più che una classe astratta vuota.

Quindi immagino sia una questione di gusti.

Un avvertimento; Se si utilizzano molte classi con proxy, come con Spring e alcuni framework di testing, una gerarchia di interfaccia parallela potrebbe causare errori meno imprevisti.

Answer 6

Le classi astratte vuote non hanno alcun senso per me, le classi astratte dovrebbero essere utilizzate per ereditare qualche comportamento. Quindi, io tendo ad essere d'accordo, è un design piuttosto brutto e un uso pessimo delle classi astratte, le interfacce marker dovrebbero essere preferite. Quindi hai due opzioni:

1. Attendi fino a quando non è necessario estendere un'altra classe per un'esigenza reale e arrabbiarsi con la classe astratta per sostituirla con un'interfaccia.
2. Non aspettare e fissare il design.

In questa particolare situazione, è vero che il design attuale non fa davvero male, per ora, quindi puoi conviverci. Tuttavia, penso che la sostituzione di queste classi astratte sia un refactoring piuttosto semplice (renderle interfacce e sostituisce extends con implements in cui si verificano errori di compilazione) e davvero non riesco a vedere cosa si potrebbe spezzare, quindi mi piacerebbe farlo.

Personalmente, e le persone potrebbero ovviamente non essere d'accordo, non mi piace essere troppo difensivo. Con regole come se non è rotto, non aggiustarlo, non aggiusterete mai nulla ("i miei test passano, perché dovrei refactoring?"). Il blocco del codice non è sicuramente la soluzione giusta, il test in modo aggressivo è la soluzione giusta.

Answer 7

Secondo la teoria di programmazione orientata agli oggetti, lo scopo principale dell'ereditarietà è il polimorfismo, il riutilizzo del codice e l'incapsulamento. Una classe astratta vuota (e quando dico questo intendo veramente vuoto, nessun costruttore, nessun metodo, nessuna proprietà, niente!) Non raggiunge nessuno dei tre obiettivi sperati da questa tecnica di programmazione. È l'equivalente a if(true){...}. cambiarlo in un'interfaccia non lo rende davvero migliore.

Se si desidera refactoring del codice, consiglierei di pensare nella direzione opposta a quella che si sta pensando, ciò che intendo con questo è: provare a astrarre proprietà, metodi e costruttori di tutte le classi che condividono un abstract classe genitore.

Questo è un duro lavoro con poca ricompensa nel breve termine, ma aumenta drasticamente la manutenibilità del codice poiché un cambiamento logico di base dovrebbe essere fatto una sola volta. Sospetto che la ragione per usare queste classi astratte vuote sia identificare un gruppo di classi che devono condividere qualcosa in comune altrimenti quale sarebbe la differenza tra Object e la classe astratta

Answer 8

Se si ha il seguente schema lo si troverà a essere il più flessibile:

interface Fooable 
{ 
    void foo(); 
    void bar(); 
} 

abstract class AbstractFoo 
    implements Fooable 
{ 
} 

class Foo 
    extends AbstractFoo 
{ 
    public void foo() 
    { 
    } 

    public void bar() 
    { 
    } 
} 

questo modo si può sempre passare attraverso l'interfaccia, ma se in seguito si scopre che si dispone di codice che può essere comune si può mettere nella classe astratta, senza dover apportare modifiche a tutti le classi.

A meno che tu non abbia una buona ragione per non farlo (e sospetto che non lo fai in questo caso) suggerirei di usarlo. Questo può finire con classi astratte vuote, ma penso che sia ok (un po 'strano, ma ok).

Se davvero non ci sono metodi nell'interfaccia o solo uno, salterò la classe astratta.

Da un punto di vista del compilatore/funzionale non vi è alcuna reale differenza tra un'interfaccia e una classe astratta in cui tutti i metodi sono astratti. L'interfaccia sarà più flessibile della classe astratta e l'interfaccia + classe astratta sarà la più flessibile di tutte.

Se fosse il mio codice mi piacerebbe fare il cambiamento a loro di essere interfacce ...

Answer 9

Le domande ovvie sono, Perché è stato messo lì? e come viene utilizzato?

La mia ipotesi è che, o (a) questa è la traccia di una falsa partenza. Una bozza iniziale aveva alcuni dati o funzioni nella classe astratta, ma mentre il programmatore lavorava, questi venivano spostati altrove, finché non rimaneva altro che un guscio vuoto. Ma più probabilmente (b) Ad un certo punto c'è un codice che dice "if (x instanceof ...". Penso che questo sia un cattivo design, fondamentalmente usando l'appartenenza a una classe come una bandiera.Se hai bisogno di una bandiera, crea una bandiera, non fingere di essere "più orientato agli oggetti" creando una classe o un'interfaccia e quindi usandola come una bandiera, che potrebbe adattarsi ad una definizione di codice migliore di codifica ma in realtà rende il codice più confuso.

+1 a Monachus per indicare che un'interfaccia vuota non è migliore di una classe astratta vuota Sì, sì, so che Sun ha fatto tutto da solo con Cloneable, ma penso che fosse una soluzione zoppa per il problema

Answer 10

La chiave è che puoi estendere solo dalla classe astratta a, mentre puoi ment altre interfacce.

Apparentemente la desi- dizione di progettazione "classe astratta vuota" è stata realizzata in modo da impedire alla classe di implementazione di estendersi da un'altra classe.

Se fossi stato io lo avrei lasciato andare, altrimenti si sarebbe rotto. La cosa migliore è ancora contattare gli sviluppatori originali e chiedere ragionamenti (e aggiungerli come commenti nella classe astratta per la tua e futura comodità).

Answer 11

Mentre non riesco a ottenere quello che sarà nella tabella in cui è mappata la classe vuota, se la classe serve a qualche scopo, beh, mantienila, finché non hai l'opportunità di rifattorizzarne alcuni.

Quello che farei sicuramente è: scrivere un commento sul motivo per cui questa classe esiste nel file. Un grande motivo per il codice pulito e "bello" è quello di non pensare ad altri sviluppatori. Un commento può aiutare con questo, anche se il codice non è bello come potrebbe essere.