Come dovrei rifattorizzare le dichiarazioni di switch come questa (Attivando il tipo) per essere più OO?

7

Vedo un codice come questo nella nostra base di codice e voglio refactoring:

(segue il typescript psuedocode):

class EntityManager{

private findEntityForServerObject(entityType:string, serverObject:any):IEntity {

    var existingEntity:IEntity = null;

    switch(entityType) {
      case Types.UserSetting:
            existingEntity = this.getUserSettingByUserIdAndSettingName(serverObject.user_id, serverObject.setting_name);
                break;

        case Types.Bar:
            existingEntity = this.getBarByUserIdAndId(serverObject.user_id, serverObject.id);
            break;

        //Lots more case statements here...
    }
    return existingEntity;
 }

 }

I lati negativi del tipo di accensione sono auto-esplicativi. Normalmente, quando si cambia comportamento in base al tipo, cerco di spingere il comportamento in sottoclassi in modo da poterlo ridurre a una singola chiamata di metodo e lasciare che il polimorfismo si occupi del resto.

Tuttavia, le seguenti due cose mi stanno facendo fermare:

1) Non voglio accoppiare il serverObject con la classe che sta memorizzando tutti questi oggetti. Non sa dove cercare le entità di un certo tipo. E sfortunatamente, l'identità di un tipo di ServerObject varia a seconda del tipo di ServerObject. (Quindi a volte è solo un ID, altre volte è una combinazione di un ID e una stringa identificativa univoca, ecc.). E questo comportamento non appartiene laggiù a quelle sottoclassi. È responsabilità di EntityManager e dei suoi delegati.

2) In questo caso, non posso modificare le classi ServerObject poiché sono semplici oggetti di dati vecchi. Va detto che ho altre istanze del metodo sopra che prendono un parametro come "IEntity" e procedono a fare quasi la stessa cosa (ma modificano leggermente il nome dei metodi che stanno chiamando per ottenere l'identità del entità). Quindi, potremmo avere:

        case Types.Bar:
            existingEntity = this.getBarByUserIdAndId(entity.getUserId(), entity.getId());
            break;

Quindi in questo caso, posso cambiare l'interfaccia dell'entità e le sottoclassi, ma questo non è un comportamento che appartiene a quella classe.

Quindi, penso che mi indica una specie di mappa. Quindi alla fine chiamerò:

private findEntityForServerObject(entityType:string, serverObject:any):IEntity {

    return aMapOfSomeSort[entityType].findByServerObject(serverObject);

}

private findEntityForEntity(someEntity:IEntity):IEntity {
    return aMapOfSomeSort[someEntity.entityType].findByEntity(someEntity);
}

Il che significa che ho bisogno di registrare una sorta di classi / funzioni di strategia in fase di esecuzione con questa mappa. E ancora, mi raccomando bene ricordati di registrarne uno per ciascun mio tipo, o otterrò un'eccezione di runtime.

C'è un modo migliore per refactoring questo? Mi sento come se mi mancasse qualcosa di veramente ovvio qui.

    
posta Taytay 03.06.2014 - 18:22
fonte

2 risposte

2

Ogni volta che inizi a passare per tipo (o fornisci se è basato sul tipo), allora stai andando contro il modello OO. In una buona progettazione OO, la classe di implementazione dovrebbe essere nascosta dal codice chiamante.

(Ciò detto, ci sono momenti in cui è più facile fare una rapida istanza di, ma cerco di renderla rara).

Il polimorfismo è la risposta qui.

In realtà, quello che stai facendo è chiamare un'azione comune su alcune classi. Quindi, scorri un'interfaccia che definisce quella chiamata comune, quindi fai riferimento agli oggetti attraverso tale interfaccia.

In altre parole, ogni volta che mi vedo scrivere "se xxx.class" o "xxx instanceof" o "switch (xxx.class)", mi chiedo se ci sia un'interfaccia comune che posso estrarre.

Ora, può essere difficile. Se le diverse classi richiedono parametri diversi, potrebbe essere necessario utilizzare una sorta di modello di Builder. Può diventare complicato.

Ma la risposta breve, comunque, è la seguente: se stai cambiando o iffing in base al tipo di classe, dovresti invece usare il polimorfismo.

    
risposta data 03.06.2014 - 19:18
fonte
1

Vorrei mappare tra entityTypes e un oggetto che sa cosa fare con quel tipo di entità. Quindi potresti avere qualcosa di simile

private findEntityForServerObject(entityType:string, serverObject:any):IEntity {
    EntityFinder entityFinder = EntityFinder.getForEntity(entityType);
    return entityFinder.find(serverObject);
}

Spingerai quindi lo switch in EntityFinder che avrà un metodo statico lungo le seguenti righe

class EntityFinder {
    public static EntityFinder getForEntity(String entityType) {
        switch (entityType) {
            case Types.UserSetting:
               return new UserSettingsFinder();
            case Types.Bar:
               return new BarSettingsFinder();
            default:
               // Possibly better than a null pointer
               return new UnknownSettingsFinder(); 
        }
    }
}

Se introduci un nuovo tipo, dovrai ricordarti di includerlo nell'interruttore, ma spero che avrai solo un posto per farlo.

    
risposta data 03.06.2014 - 19:03
fonte

Leggi altre domande sui tag