2013-08-01 15 views
16

Sto creando un software in C#. Sto usando una classe astratta, Instruction, che ha questi pezzi di codice:Risoluzione del problema "Chiamata metodo virtuale in costruttore"

protected Instruction(InstructionSet instructionSet, ExpressionElement newArgument, 
    bool newDoesUseArgument, int newDefaultArgument, int newCostInBytes, bool newDoesUseRealInstruction) { 

    //Some stuff 

    if (DoesUseRealInstruction) { 
     //The warning appears here. 
     RealInstruction = GetRealInstruction(instructionSet, Argument); 
    } 
} 

e

public virtual Instruction GetRealInstruction(InstructionSet instructionSet, ExpressionElement argument) { 
    throw new NotImplementedException("Real instruction not implemented. Instruction type: " + GetType()); 
} 

Così ReSharper mi dice che alla linea marcata che sto 'chiamando un metodo virtuale nel costruttore' e questo è cattivo. Capisco la cosa sull'ordine in cui sono chiamati i costruttori. Tutte le sostituzioni del metodo GetRealInstruction simile a questa:

public override Instruction GetRealInstruction(InstructionSet instructionSet, ExpressionElement argument) { 
    return new GoInstruction(instructionSet, argument); 
} 

Così non dipendono da alcun dato nella classe; restituiscono solo qualcosa che dipende dal tipo derivato. (quindi l'ordine del costruttore non li influenza).

Quindi, dovrei ignorarlo? Preferirei di no; così qualcuno potrebbe mostrarmi come potrei evitare questo avvertimento?

Non riesco a utilizzare i delegati in modo ordinato perché il metodo GetRealInstruction ha un sovraccarico in più.

risposta

16

Ho riscontrato questo problema un bel po 'di volte e il modo migliore che ho trovato per risolverlo correttamente è quello di astrarre il metodo virtuale che viene chiamato dal costruttore, in una classe separata. Dovresti quindi passare un'istanza di questa nuova classe nel costruttore della tua classe astratta originale, con ogni classe derivata che passa la sua versione al costruttore base. È un po 'complicato da spiegare, quindi darò un esempio, basato sul tuo.

public abstract class Instruction 
{ 
    protected Instruction(InstructionSet instructionSet, ExpressionElement argument, RealInstructionGetter realInstructionGetter) 
    { 
     if (realInstructionGetter != null) 
     { 
      RealInstruction = realInstructionGetter.GetRealInstruction(instructionSet, argument); 
     } 
    } 

    public Instruction RealInstruction { get; set; } 

    // Abstracted what used to be the virtual method, into it's own class that itself can be inherited from. 
    // When doing this I often make them inner/nested classes as they're not usually relevant to any other classes. 
    // There's nothing stopping you from making this a standalone class of it's own though. 
    protected abstract class RealInstructionGetter 
    { 
     public abstract Instruction GetRealInstruction(InstructionSet instructionSet, ExpressionElement argument); 
    } 
} 

// A sample derived Instruction class 
public class FooInstruction : Instruction 
{ 
    // Passes a concrete instance of a RealInstructorGetter class 
    public FooInstruction(InstructionSet instructionSet, ExpressionElement argument) 
     : base(instructionSet, argument, new FooInstructionGetter()) 
    { 
    } 

    // Inherits from the nested base class we created above. 
    private class FooInstructionGetter : RealInstructionGetter 
    { 
     public override Instruction GetRealInstruction(InstructionSet instructionSet, ExpressionElement argument) 
     { 
      // Returns a specific real instruction 
      return new FooRealInstuction(instructionSet, argument); 
     } 
    } 
} 

// Another sample derived Instruction classs showing how you effictively "override" the RealInstruction that is passed to the base class. 
public class BarInstruction : Instruction 
{ 
    public BarInstruction(InstructionSet instructionSet, ExpressionElement argument) 
     : base(instructionSet, argument, new BarInstructionGetter()) 
    { 
    } 

    private class BarInstructionGetter : RealInstructionGetter 
    { 
     public override Instruction GetRealInstruction(InstructionSet instructionSet, ExpressionElement argument) 
     { 
      // We return a different real instruction this time. 
      return new BarRealInstuction(instructionSet, argument); 
     } 
    } 
} 

Nel vostro esempio particolare lo fa arrivare un po 'di confusione e ho iniziato a corto di nomi sensati, ma questo è dovuto al fatto che già hanno un annidamento di istruzioni all'interno di Istruzioni, vale a dire un istruzione ha un RealInstruction (o almeno lo fa opzionalmente); ma come puoi vedere è ancora possibile ottenere ciò che desideri ed evitare chiamate virtuali da un costruttore.

Nel caso non sia ancora chiaro, darò anche un esempio basato su uno che ho usato di recente nel mio codice. In questo caso ho 2 tipi di moduli, un modulo di intestazione e un modulo di messaggio, entrambi i quali ereditano da un modulo di base. Tutti i moduli hanno campi ma ogni tipo di modulo ha un meccanismo diverso per la costruzione dei suoi campi, quindi in origine avevo un metodo astratto chiamato GetOrderedFields che ho chiamato dal costruttore base e il metodo era sovrascritto in ogni classe di form derivata. Questo mi ha dato l'avvertimento del resharper che hai citato. La mia soluzione era lo stesso schema di cui sopra ed è il seguente

internal abstract class FormInfo 
{ 
    private readonly TmwFormFieldInfo[] _orderedFields; 

    protected FormInfo(OrderedFieldReader fieldReader) 
    { 
     _orderedFields = fieldReader.GetOrderedFields(formType); 
    } 

    protected abstract class OrderedFieldReader 
    { 
     public abstract TmwFormFieldInfo[] GetOrderedFields(Type formType); 
    } 
} 

internal sealed class HeaderFormInfo : FormInfo 
{ 
    public HeaderFormInfo() 
     : base(new OrderedHeaderFieldReader()) 
    { 
    } 

    private sealed class OrderedHeaderFieldReader : OrderedFieldReader 
    { 
     public override TmwFormFieldInfo[] GetOrderedFields(Type formType) 
     { 
      // Return the header fields 
     } 
    } 
} 

internal class MessageFormInfo : FormInfo 
{ 
    public MessageFormInfo() 
     : base(new OrderedMessageFieldReader()) 
    { 
    } 

    private sealed class OrderedMessageFieldReader : OrderedFieldReader 
    { 
     public override TmwFormFieldInfo[] GetOrderedFields(Type formType) 
     { 
      // Return the message fields 
     } 
    } 
} 
+0

Ho dimenticato di menzionare, ma un altro vantaggio in questo modo è che nel tuo caso eviti la necessità di avere il metodo base virtuale, piuttosto che astratto, quindi ottieni un controllo in fase di compilazione invece di lanciare un'eccezione (come menzionato @TarasDzyoba) . – Richard

+0

Questo è molto ben pensato, grazie. Alla fine ho evitato questo problema in un altro modo, ma questa è un'ottima soluzione per il futuro. –

1

Si potrebbe passare nel vero istruzioni nel costruttore della classe base:

protected Instruction(..., Instruction realInstruction) 
{ 
    //Some stuff 

    if (DoesUseRealInstruction) { 
     RealInstruction = realInstruction; 
    } 
} 

public DerivedInstruction(...) 
    : base(..., GetRealInstruction(...)) 
{ 
} 

Oppure, se si vuole veramente chiamare una funzione virtuale dal costruttore (che vivamente si discorage da) si potrebbe sopprimere la warning ReSharper:

// ReSharper disable DoNotCallOverridableMethodsInConstructor 
    RealInstruction = GetRealInstruction(instructionSet, Argument); 
// ReSharper restore DoNotCallOverridableMethodsInConstructor 
+0

Che funzionerebbe bene, ma non rimuoverebbe il motivo dell'avvertimento, nascondilo. Potrei usarlo se non ci sono altre soluzioni. Mi piace lavorare una volta e mettere le cose a posto. Non mi piace dare l'impostazione al chiamante. –

+0

Risposta modificata. Ma immagino che non sia ancora quello che stai cercando. E dubito che ci sia una soluzione del genere. C'è [una buona ragione] (http://stackoverflow.com/questions/119506/virtual-member-call-in-a-constructor?rq=1) per l'avviso ReSharper. –

+0

Capisco. Ho finito per spostare la chiamata 'GetRealInstruction' su un'altra funzione, in modo che venga chiamata solo quando è necessaria. –

2

Quando si crea un'istanza della classe derivata, il vostro stack di chiamate sarà simile a questa:

GetRealInstruction() 
BaseContructor() 
DerivedConstructor() 

GetRealInstruction viene sovrascritto nella classe derivata, il cui costruttore non ha ancora finito di funzionare.

Non so come apparirà l'altro codice, ma in primo caso è necessario verificare se davvero ci fosse bisogno di una variabile membro. Hai un metodo che restituisce l'oggetto che ti serve. Se davvero ne hai bisogno, crea una proprietà e chiama lo GetRealInstruction() nel getter.

Inoltre è possibile rendere astratto GetRealInstruction. In questo modo non devi lanciare l'eccezione e il compilatore ti darà un errore se ti dimentichi di sovrascriverlo in una classe derivata.

+0

Capisco il problema dell'ordine chiamante. Ho solo bisogno di GetRealInstruction in alcune classi derivate (quelle con il set di bit doesRequireRealInstruction) quindi non dovrei renderlo astratto. Non voglio davvero creare una proprietà per questo perché mi piace la differenza di accesso/operazione tra campi e metodi. (Io uso le proprietà in casi come questo, ma per operazioni molto più semplici) ho finito per spostare la chiamata da qualche altra parte. –

2

È possibile introdurre un'altra classe astratta RealInstructionBase modo che il codice sarà simile:

public abstract class Instruction { 
    public Instruction() { 
     // do common stuff 
    } 
} 

public abstract class RealInstructionBase : Instruction { 
    public RealInstructionBase() : base() { 
     GetRealInstruction(); 
    } 

    protected abstract object GetRealInstruction(); 
} 

Ora ogni istruzione che ha bisogno di usare RealInstruction deriva dal RealInstructionBase e tutti gli altri derivano da istruzioni. In questo modo dovresti averli tutti inizializzati correttamente.

MODIFICA: Ok, questo vi darà solo un design più pulito (no se nel costruttore) ma non elimina l'avviso. Ora, se si desidera sapere perché si riceve l'avviso, è possibile fare riferimento a this question. Fondamentalmente il punto è che sarai al sicuro quando marchierai le tue classi dove stai implementando il metodo astratto come sigillato.

+0

Questo è certamente più pulito. Tuttavia, ho deciso di risolverlo spostando la chiamata 'GetRealInstruction' appena prima che sia necessaria, non nel costruttore. Questa funzione non è troppo costosa. –

0

Un'altra opzione è quella di introdurre un metodo di Initialize(), dove si fa tutto l'inizializzazione che richiede un oggetto completamente costruito.

Problemi correlati