Come migliorare il codice che deve gestire gli oggetti elenco in casi generali e speciali?

Question

Come migliorare il codice che deve gestire gli oggetti elenco in casi generali e speciali?

#1 da (3 voti)

0

Ho un problema di implementazione che ho condensato nel seguente frammento di codice, perché il codice reale è molto più complicato. Il problema principale è che ho un oggetto contenitore con un elenco misto di persone, ad esempio Manager e ingegneri:

interface Person {

}

class Manager implements Person {

}

class Engineer implements Person {

}

Nella maggior parte dei casi devo accedere all'elenco delle persone senza conoscere la sottoclasse concreta di una persona. Ma in alcuni metodi devo conoscere il tipo concreto dei membri, ad esempio per contare i membri o per aggiungere i membri. In entrambe le implementazioni ho un brutto codice:

Nell'implementazione A è il codice per separare i tipi di persone per il conteggio.

class EnterpriseA {

    private final List<Person> members = new ArrayList<Person>();

    public void addMember(Person person) {
        this.members.add(person);
    }

    public void doSomethingWithManagers() {
        // separate by instanceof
    }

    public void doSomethingWithEngineers() {
        // separate by instanceof
    }
}

Nell'implementazione B è il codice per aggiungere una persona all'elenco di Manager o Ingegnere.

class EnterpriseB {

    private final List<Manager> managers = new ArrayList<Manager>();

    private final List<Engineer> engineers = new ArrayList<Engineer>();

    public void addMember(Person person) {
        if (person instanceof Manager) {
            this.managers.add((Manager) person);
        } else if (person instanceof Engineer) {
            this.engineers.add((Engineer) person);
        }
    }

    public void doSomethingWithManagers() {
        // iterate over the managers.
    }

    public void doSomethingWithEngineers() {
        // iterate over the engineers.
    }
}

Come posso migliorare il codice ed evitare il brutto codice in addMember () e countManagers () rispettivamente? I suggerimenti intelligenti sono ben accetti.
Quale alternativa preferiresti? Si prega di fornire dei motivi, in quali casi si preferirebbe quale soluzione.

clean-code class-design

posta shylynx 09.10.2014 - 14:06

fonte

1 risposta

Leggi altre domande sui tag clean-code class-design

Come si chiamano le linee del codice che dichiarano che una classe o una funzione è un modello? [chiuso] L'algoritmo giusto per ottenere una combinazione perfetta

score 3 · Answer 1

Se questi tipi sono correlati, come nel tuo esempio, direi che la soluzione A è superiore.

Utilizza il polimorfismo - il fatto che Manager e Employee siano entrambi Person s, quindi potresti es. creare un elenco di tutti i nomi o riassumere i loro stipendi senza forzare il codice a preoccupare (o anche sapere) le distinzioni dei titoli di lavoro.

I controlli di tipo "Manuale" ( instanceof ) in soluzione B sono in genere un odore di codice e un dolore quando si tratta di manutenzione.

È anche più bug-prono - anche il tuo codice di esempio conteneva già un bug (confusione di un ingegnere con un manager), che ho già corretto.

Se dovessi aggiungere un altro tipo, ad esempio Accountant , invece di aggiungere solo countAccountants , devi inserire un altro controllo if in addMember , e possibilmente vari altri metodi.

Sì, potrebbero esserci problemi di rendimento per quanto riguarda la soluzione A . Dovendo filtrare la lista e ricomprare tutto Managers ogni volta potrebbe essere inefficiente per le grandi liste.

Ma c'è una soluzione per questo: puoi mantenere il conteggio nella classe e aggiornare i contatori nel tuo metodo addMember , così come qualsiasi altro metodo che modifica il contenuto della tua raccolta (come removeMember ), il tipo di memorizzazione nella cache i dati.

Potresti persino renderlo più generico e creare un Map<Class<? extends Person>, Integer> typeToCount , in cui ogni chiave ( Employee.class , Manager.class ) avrebbe un valore corrispondente che rappresenta il numero di oggetti di questo particolare tipo memorizzati. Ciò ridurrebbe ulteriormente lo sforzo di mantenere più tipi e proteggerti dagli errori di scansione.

Così:

class EnterpriseAPlus {
  private final List<Person> members = new ArrayList<Person>();
  private final Map<Class<? extends Person>, Integer> countByType = 
        new HashMap<Class<? extends Person>, Integer>();

  public void addMember(Person person) {
     members.add(person);
     // (thread safety concerns disregarded for the sake of simplicity)
     Class<? extends Person> personClass = person.getClass();
     int currentCount;
     if (countByType.containsKey(personClass)) {
        currentCount = countByType.get(personClass);
     } else {
        currentCount = 0;
     }
     countByType.put(personClass, currentCount + 1);
  }

  public int countEngineers() {
     return getCountOf(Engineer.class);
  }

  public int countManagers() {
     return getCountOf(Manager.class);
  }

  protected final int getCountOf(Class<? extends Person> personClass) {
     if (!countByType.containsKey(personClass)) {
        return 0;
     }
     return countByType.get(personClass);
  }
}

Devi gestire Accountant s ora? Basta aggiungere:

public int countAccountants() {
    return getCountOf(Accountant.class);
}

Non è più necessario fare confusione con addMember . Potresti anche aggiungere countAccountants sottoclassando EnterpriseAPlus - ricorda il principio aperto / chiuso ...