2013-08-01 6 views
16

私はC#でソフトウェアを作っています。だから、ReSharperのは、「コンストラクタで仮想メソッドを呼び出す」マークされた行で私はと言われます'コンストラクタ内の仮想メソッド呼び出し'の問題を解決する

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); 
    } 
} 

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

:私は、コードのこれらのビットを持つ抽象クラス、Instructionを、使用していますこれは悪いことです。私は、コンストラクタが呼び出される順序についてのことを理解しています。 GetRealInstruction方法のすべてのオーバーライドは、次のようになります。

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

そこで彼らは、クラス内の任意のデータには依存しません。派生した型に依存するものを返すだけです。 (コンストラクタの順序はそれらに影響しません)。

私は無視する必要がありますか?私はむしろないと思います;誰も私にこの警告を避ける方法を教えてもらえますか?

GetRealInstructionメソッドにオーバーロードがもう1つあるため、代理人をきれいに使用することはできません。

答えて

16

私はこの問題をかなり数回遭遇しました。これを適切に解決するための最良の方法は、コンストラクタから呼び出されている仮想メソッドを別のクラスに抽象化することです。次に、この新しいクラスのインスタンスを元の抽象クラスのコンストラクタに渡します。各抽象クラスは、独自のバージョンを基本コンストラクタに渡します。説明するのはちょっと難しいので、私はあなたに基づいて例を挙げます。あなたの特定の例では

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); 
     } 
    } 
} 

それは少し混乱するかもしれないし、私は賢明な名前が不足し始めたが、これは事実にすでに命令がR​​ealInstructionを持っているつまり、取扱説明書内の命令のネストを持っているためです(または少なくとも任意に);見ることができるように、あなたが望むものを達成し、コンストラクターからの仮想メンバー呼び出しを避けることはまだ可能です。

それでも明らかでない場合は、最近自分のコードで使用した例に基づいて例を挙げます。この場合、私はヘッダーフォームとメッセージフォームの2種類のフォームを持ち、どちらも基本フォームを継承しています。すべてのフォームにはフィールドがありますが、各フォームタイプにはフィールドを構成するメカニズムが異なります。元々、基本コンストラクターから呼び出されたGetOrderedFieldsという抽象メソッドがあり、そのメソッドは各フォームクラスでオーバーライドされました。これは私にあなたが言及するresharperの警告を与えました。

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

私は言及を忘れましたが、この方法のもう一つの利点は、あなたの場合、抽象メソッドではなく仮想メソッドを仮想化する必要がなくなるため、例外をスローするのではなくコンパイル時のチェックを得ることです(@TarasDzyobaが述べたように) 。 – Richard

+0

これは非常によく考えられています、ありがとうございます。私は最終的にこの問題を別の方法で回避しましたが、これは将来的には非常に良い解決策です。 –

1

あなたは、基本クラスのコンストラクタに、実際の命令に渡すことができます:あなたは本当に(私は非常にあなたをdiscorageた)あなたのコンストラクタから仮想関数を呼び出したい場合は抑えることができ、

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

    if (DoesUseRealInstruction) { 
     RealInstruction = realInstruction; 
    } 
} 

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

かReSharperの警告:あなたはあなたの派生クラスのインスタンスを作成するとき

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

これはうまくいくはずですが、警告の理由が取り除かれず、単に非表示になります。私は他の解決策がない場合、それを使うことができます。私は一度働くのが好きで、自分自身を立てるように物事を設定したい私は発信者にセットアップを与えるのが好きではありません。 –

+0

回答が修正されました。しかし、私はそれがまだあなたが探しているものではないと思う。そして、私はそのような解決策があるのか​​疑問です。 ReSharperの警告には[正当な理由があります(http://stackoverflow.com/questions/119506/virtual-member-call-in-a-constructor?rq=1)。 –

+0

私は理解しています。私は 'GetRealInstruction'呼び出しを別の関数に移してしまったので、必要なときに呼び出されます。 –

2

、あなたのコールスタックは次のようになります。

GetRealInstruction() 
BaseContructor() 
DerivedConstructor() 

GetRealInstructionは、コンストラクタがまだ実行されていない派生クラスでオーバーライドされます。

あなたの他のコードがどのように見えるかはわかりませんが、まずこの場合にメンバー変数が本当に必要かどうか確認してください。必要なオブジェクトを返すメソッドがあります。あなたが本当にそれを必要とするなら、財産を作り、GetRealInstruction()にゲッターで電話してください。

また、GetRealInstructionを抽象的にすることもできます。そうすれば、例外をスローする必要はなく、派生クラスでオーバーライドするのを忘れた場合、コンパイラはエラーを返します。

+0

私は発注の問題を理解しています。特定の派生クラス(doesRequireRealInstructionビットが設定されたクラス)でGetRealInstructionを必要とするだけなので、抽象化しないでください。フィールドとメソッドのアクセス/操作の違いが好きなので、私は実際にこれを行うためのプロパティを作成したくありません。 (私はこのような場合にプロパティを使用しますが、はるかに簡単な操作のために)コールをどこか別の場所に移動しました。 –

2

あなたのコードは次のようになりますので、あなたは別の抽象クラスRealInstructionBaseを導入することができますRealInstructionはRealInstructionBaseから派生し、他のすべての命令から派生使用する必要がある

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

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

    protected abstract object GetRealInstruction(); 
} 

今、各命令。このようにして、すべてを正しく初期化する必要があります。

EDIT:これで、よりクリーンなデザイン(コンストラクタ内にある場合のみ)が得られますが、警告を取り除くことはありません。 最初に警告が表示される理由を知りたい場合は、this questionを参照してください。基本的には、抽象メソッドを実装しているクラスを封印してマークすると安全です。

+0

それは確かにきれいです。しかし、私は 'GetRealInstruction'呼び出しを、コンストラクタではなく、必要になる直前に動かすことで、それを整理しました。この機能はそれほど高価ではありません。 –

0

別のオプションを次のように私のソリューションは、上記と同じパターンだったとされますが、完全に構築されたオブジェクトを必要とするすべての初期化を行うInitialize()方法を、導入することです。

関連する問題