Pratica buona o cattiva per mascherare le raccolte Java con nomi di classi significativi?

43

Ultimamente avevo l'abitudine di "mascherare" le raccolte Java con nomi di classe rispettosi dell'ambiente. Alcuni semplici esempi:

// Facade class that makes code more readable and understandable.
public class WidgetCache extends Map<String, Widget> {
}

o

// If you saw a ArrayList<ArrayList<?>> being passed around in the code, would you
// run away screaming, or would you actually understand what it is and what
// it represents?
public class Changelist extends ArrayList<ArrayList<SomePOJO>> {
}

Un collega mi ha fatto notare che questa è una cattiva pratica e introduce ritardo / latenza, oltre a essere un anti-modello OO. Posso capire che introduce un piccolo livello di overhead delle prestazioni, ma non riesco a immaginare che sia del tutto significativo. Quindi chiedo: è buono o cattivo farlo e perché?

    
posta herpylderp 27.06.2014 - 20:48
fonte

6 risposte

73

Lag / latenza? Chiamo BS su questo. Ci dovrebbe essere esattamente zero overhead da questa pratica. ( Modifica: è stato sottolineato nei commenti che questo può, di fatto, inibire le ottimizzazioni eseguite dalla macchina virtuale HotSpot. Non ne so abbastanza sull'implementazione VM per confermare o negare questo. basando il mio commento sull'implementazione di funzioni virtuali in C ++.)

C'è un sovraccarico di codice. Devi creare tutti i costruttori dalla classe base che desideri, inoltrandone i parametri.

Anche io non lo vedo come un anti-modello, di per sé. Tuttavia, lo vedo come un'opportunità persa. Invece di creare una classe che deriva la classe base solo per il gusto di rinominare, come su di te invece crei una classe che contiene la collezione e offre un'interfaccia migliorata caso per caso? La cache del tuo widget dovrebbe davvero offrire l'interfaccia completa di una mappa? O dovrebbe invece offrire un'interfaccia specializzata?

Inoltre, nel caso delle collezioni, il pattern semplicemente non funziona insieme alla regola generale di usare le interfacce, non le implementazioni - cioè, nel codice di raccolta semplice, si creerebbe un HashMap<String, Widget> , e poi lo si assegnerà a una variabile di tipo Map<String, Widget> . Il tuo WidgetCache non può estendere Map<String, Widget> , perché è un'interfaccia. Non può essere un'interfaccia che estende l'interfaccia di base, perché HashMap<String, Widget> non implementa quell'interfaccia e nessuna altra raccolta standard. E mentre puoi renderlo una classe che estende HashMap<String, Widget> , devi quindi dichiarare le variabili come WidgetCache o Map<String, Widget> , e il primo ti perde la flessibilità di sostituire una collezione diversa (forse la pigra collezione di caricamento di ORM) , mentre il secondo tipo di sconfigge il punto di avere la classe.

Alcuni di questi contrappunti si applicano anche alla mia classe specializzata proposta.

Questi sono tutti punti da considerare. Potrebbe o potrebbe non essere la scelta giusta. In entrambi i casi, gli argomenti offerti dal collega non sono validi. Se pensa che sia un anti-modello, dovrebbe nominarlo.

    
risposta data 27.06.2014 - 20:58
fonte
23

Secondo IBM questo in realtà è un anti-modello. Queste classi simili a "typedef" sono chiamate tipi di psuedo.

L'articolo lo spiega molto meglio di me, ma cercherò di riepilogarlo nel caso in cui il link si interrompa:

  • Qualsiasi codice che si aspetta un WidgetCache non può gestire un Map<String, Widget>
  • Questi pseudotipi sono "virali" quando si usano più pacchetti che portano a incompatibilità mentre il tipo di base (solo una stupida mappa < ... >) avrebbe funzionato in tutti i casi in tutti i pacchetti.
  • I tipi di pseudo sono spesso concreti, non implementano interfacce specifiche perché le loro classi base implementano solo la versione generica.

Nell'articolo propongono il seguente trucco per semplificare la vita senza usare pseudo tipi:

public static <K,V> Map<K,V> newHashMap() {
    return new HashMap<K,V>(); 
}

Map<Socket, Future<String>> socketOwner = Util.newHashMap();

Che funziona a causa dell'inferenza di tipo automatica.

(Sono arrivato a questa risposta tramite questa domanda di overflow dello stack correlato)

    
risposta data 28.06.2014 - 10:37
fonte
13

Il risultato in termini di prestazioni sarebbe limitato al massimo a una ricerca vtable, che probabilmente stai già subendo. Non è un valido motivo per opporsi.

La situazione è abbastanza comune che la maggior parte di tutti i linguaggi di programmazione tipizzati staticamente ha una sintassi speciale per i tipi di aliasing, solitamente chiamata typedef . Probabilmente Java non li ha copiati perché inizialmente non aveva tipi parametrizzati. Estendere una classe non è l'ideale, a causa delle ragioni per cui Sebastian ha risposto così bene nella sua risposta, ma può essere una soluzione ragionevole per la sintassi limitata di Java.

I typedef hanno una serie di vantaggi. Esprimono l'intento del programmatore più chiaramente, con un nome migliore, ad un livello più appropriato di astrazione. Sono più facili da cercare per scopi di debug o di refactoring. Prendi in considerazione la possibilità di trovare ovunque un WidgetCache utilizzato rispetto alla ricerca di quegli usi specifici di Map . Sono più facili da modificare, ad esempio se in un secondo momento hai bisogno di un LinkedHashMap o persino del tuo contenitore personalizzato.

    
risposta data 27.06.2014 - 21:36
fonte
11

Ti suggerisco, come altri già menzionato, di usare la composizione sull'ereditarietà, in modo da poter esporre solo i metodi realmente necessari, con nomi che corrispondono al caso d'uso previsto. Gli utenti della tua classe hanno davvero bisogno di sapere che WidgetCache è una mappa? E riuscire a farne tutto ciò che vogliono? O hanno solo bisogno di sapere che è una cache per i widget?

Esempio di classe del mio codebase, con soluzione a problemi simili che hai descritto:

public class Translations {

    private Map<Locale, Properties> translations = new HashMap<>();

    public void appendMessage(Locale locale, String code, String message) {
        /* code */
    }

    public void addMessages(Locale locale, Properties messages) {
        /* code */
    }

    public String getMessage(Locale locale, String code) {
        /* code */
    }

    public boolean localeExists(Locale locale) {
        /* code */
    }
}

Puoi vedere che internamente è "solo una mappa", ma l'interfaccia pubblica non lo mostra. E ha metodi "programmabili" come appendMessage(Locale locale, String code, String message) per un modo più semplice e più significativo di inserire nuove voci. E gli utenti di classe non possono fare, ad esempio, translations.clear() , perché Translations non sta estendendo Map .

Se lo desideri, puoi sempre delegare alcuni dei metodi necessari alla mappa utilizzata internamente.

    
risposta data 28.06.2014 - 17:30
fonte
6

Lo vedo come un esempio di un'astrazione significativa. Una buona astrazione ha un paio di tratti:

  1. Nasconde i dettagli di implementazione che sono irrilevanti per il codice che la consuma.

  2. È solo complesso come deve essere.

Estendendo, stai esponendo l'intera interfaccia del genitore, ma in molti casi molto potrebbe essere nascosto meglio, quindi dovresti fare ciò che suggerisce Sebastian Redl e favorisci la composizione sull'ereditarietà e aggiungi un'istanza del genitore come membro privato della tua classe personalizzata. Qualsiasi metodo di interfaccia che abbia senso per la tua astrazione può essere facilmente delegato a (nel tuo caso) la raccolta interna.

Per quanto riguarda l'impatto sulle prestazioni, è sempre consigliabile ottimizzare il codice per verificarne la leggibilità e, se si sospetta un impatto sulle prestazioni, profila il codice per confrontare le due implementazioni.

    
risposta data 27.06.2014 - 21:37
fonte
4

+1 alle altre risposte qui. Aggiungerò anche che in realtà è considerata una pratica molto buona dalla community Domain Driven Design (DDD). Sostengono che il dominio e le interazioni con esso dovrebbero avere un significato di dominio semantico rispetto alla struttura dei dati sottostante. Un Map<String, Widget> potrebbe essere una cache, ma potrebbe anche essere qualcos'altro, ciò che hai fatto correttamente In My Not So Humble Opinion (IMNSHO) è quello di modellare ciò che rappresenta la raccolta, in questo caso una cache.

Aggiungerò una modifica importante in quanto il wrapper della classe dominio attorno alla struttura dati sottostante dovrebbe probabilmente avere anche altre variabili o funzioni membro che rendono veramente una classe dominio con interazioni rispetto a una semplice struttura dati (se solo Java aveva i tipi di valore, li otterremo in Java 10 - promessa!)

Sarà interessante vedere quale impatto avranno gli stream di Java 8 su tutto ciò, posso immaginare che forse alcune interfacce pubbliche preferiranno trattare con un flusso di (inserire primitive Java o String comuni) rispetto a Java oggetto.

    
risposta data 27.06.2014 - 21:59
fonte

Leggi altre domande sui tag