Problema di progettazione - L'ereditarietà è il modo giusto per semplificare questo codice?

Question

Ho un problema di progettazione che vorrei risolvere. Ho un'interfaccia, chiamiamolo IProtocol, che è implementato da due classi separate. Stiamo guardando oltre 600 linee di codice qui. La stragrande maggioranza delle cose che fanno è la stessa, tranne per alcune aree specifiche, come DiffStuff();

struttura attuale è qualcosa di simile:

public class Protocol1 : IProtocol 
{ 
    MyInterfaceMethod1() 
    { 
    Same1(); 
    DiffStuff(); 
    Same2(); 
    } 
} 

E

public class Protocol2 : IProtocol 
{ 
    MyInterfaceMethod1() 
    { 
    Same1(); 
    Same2(); 
    } 
} 

Mi preoccupo di avere errori di copia-incolla t è un classico problema di duplicazione del codice se tengo separati i due protocolli. Stiamo parlando di 600 righe di codice complete ciascuna, non di semplici metodi.

Sto pensando di cambiare l'implementazione di protocollo1 ereditare da Protocollo2, come questo (Protocollo2 sarebbe per lo più rimanere lo stesso, tranne che avrei dovuto avvolgere Same1() e Same2() in metodi privati.)

public class Protocol1 : Protocol2 
{ 
    void Same1() 
    { 
    base.Same1(); 
    } 

    void Same2() 
    { 
    base.Same2(); 
    } 

    MyInterfaceMethod1() 
    { 
    Same1(); 
    DiffStuff(); 
    Same2(); 
    } 
} 

È questo il modo giusto per affrontare questo problema?

Modifica: Molte persone mi hanno aiutato con questa domanda, grazie per la chiara comprensione. Nel mio caso, i due oggetti non sono dello stesso tipo, anche se gran parte della loro implementazione è condivisa, quindi sono andato con Bobby's suggestion per usare la classe base astratta, creando piccoli metodi per incapsulare le modifiche tra le classi. Ulteriori Grazie a:

• jloubert
• Hans Passant
• Jeff sternale

Answer 1

/// <summary> 
    /// IProtocol interface 
    /// </summary> 
    public interface IProtocol 
    { 
     void MyInterfaceMethod1(); 
     void Same1(); 
     void Same2(); 
    } 

poi ...

public abstract class ProtocolBase : IProtocol 
{ 
    #region IProtocol Members 

    public void MyInterfaceMethod1() 
    { 
     // Implementation elided... 
    } 

    public void Same1() 
    { 
     // Implementation elided... 
    } 

    public void Same2() 
    { 
     // Implementation elided... 
    } 

    public abstract void DiffStuff(); 

    #endregion 
} 

finalmente ...

public sealed class Protocol1 : ProtocolBase 
{ 
    public override void DiffStuff() 
    { 
     // Implementation elided... 
    } 
} 

public sealed class Protocol2 : ProtocolBase 
{ 
    public override void DiffStuff() 
    { 
     // Implementation elided... 
    } 
} 

Answer 2

Non proprio. Non ha senso aggiungere i metodi Same1 e Same2, ereditali da ProtocolBase. E DiffStuff() dovrebbe essere un metodo virtuale in modo da poterlo sovrascrivere e dargli un comportamento diverso.

Answer 3

Sono d'accordo con MathEpic. Vorrei utilizzare il modello Template method.

Answer 4

ho una migliore progettazione (a mio parere)

public abstract class Protocol_common : IProtocol 
{ 
    MyInterfaceMethod1() 
    { 
    Same1(); 
    DiffStuff(); 
    Same2(); 
    } 

    abstract void DiffStuff(); 

} 

public class Protocol1 : Protocol_common 
{ 
    DiffStuff() 
    { 
     /// stuff here. 
    } 
} 

public class Protocol2 : Protocol_common 
{ 
    DiffStuff() 
    { 
     /// nothing here. 
    } 
} 

(Che in realtà è più pseudo-codice di corretta C#, ma mi ha colpito i punti alti)

Answer 5

Sei terribilmente vicino a descrivere la template method pattern , che ha un lungo pedigree come soluzione efficace per problemi come il tuo.

Tuttavia, si dovrebbe considerare l'utilizzo della composizione anziché dell'ereditarietà. The two approaches offer different advantages and disadvantages, ma la composizione è spesso (di solito?) Meglio ^*:

public class Protocol { 

     private ISpecialHandler specialHandler; 

     public Protocol() {} 

     public Protocol(ISpecialHandler specialHandler) { 
      this.specialHandler = specialHandler; 
     } 

     void Same1() {} 
     void Same2() {} 

     public void DoStuff() { 
      this.Same1(); 
      if (this.specialHandler != null) { 
       this.specialHandler.DoStuff(); 
      } 
      this.Same2(); 
     } 
} 

chiamanti può quindi passare a istanze di oggetti (strategies) che forniscono algoritmi specializzati per gestire qualunque caso è a portata di mano:

Protocol protocol1 = new Protocol(new DiffStuffHandler()); 
protocol1.DoStuff(); 

^* Vedi Patterns I Hate #2: Template Method per una dettagliata spiegazione del perché la composizione è solitamente migliore dell'eredità nel tuo caso.

Answer 6

Si potrebbe prendere in considerazione se questo tipo di modello funziona:

public class ProtocolHelper 
{ 
    public void Same1() {} 
    public void Same2() {} 
} 

public class Protocol1 : IProtocol 
{ 
    private readonly ProtocolHelper _helper = new ProtocolHelper(); 

    void MyInterfaceMethod1() 
    { 
     _helper.Same1(); 
     DiffStuff(); 
     _helper.Same2(); 
    } 
} 

Si può dire se questo ha un senso per vedere se si può trovare con un buon nome per la classe del 'ProtocolHelper'. Se un nome deriva naturalmente dalla tua logica, allora questo è un buon modo per rompere la classe. Potrebbe essere necessario passare alcune dipendenze (come i campi privati) come parametri ai metodi per farlo funzionare.