1405

オブジェクト コンストラクターからの仮想メンバーの呼び出しについて、ReSharper から警告が表示されます。

なぜこれがすべきではないのでしょうか？

4

18 に答える 18

1222

C# で記述されたオブジェクトが構築されると、イニシャライザが最も派生したクラスから基本クラスの順に実行され、次にコンストラクタが基本クラスから最も派生したクラスの順に実行されます (詳細については、Eric Lippert のブログを参照してください)。これがなぜであるかについて）。

また、.NET オブジェクトは構築時に型を変更しませんが、最初は最も派生した型であり、メソッド テーブルは最も派生した型用です。これは、仮想メソッド呼び出しが常に最も派生した型で実行されることを意味します。

これら 2 つの事実を組み合わせると、コンストラクターで仮想メソッド呼び出しを行い、それが継承階層で最も派生した型ではない場合、コンストラクターが呼び出されていないクラスで呼び出されるという問題が残ります。したがって、そのメソッドを呼び出すのに適した状態ではない可能性があります。

もちろん、この問題は、クラスをシールとしてマークして、継承階層で最も派生した型であることを確認すると軽減されます。この場合、仮想メソッドを呼び出すことは完全に安全です。

于 2008-09-23T07:21:13.747 に答える
727

あなたの質問に答えるために、次の質問を考えてみてください:Childオブジェクトがインスタンス化されると、以下のコードは何を出力しますか?

class Parent
{
    public Parent()
    {
        DoSomething();
    }

    protected virtual void DoSomething() 
    {
    }
}

class Child : Parent
{
    private string foo;

    public Child() 
    { 
        foo = "HELLO"; 
    }

    protected override void DoSomething()
    {
        Console.WriteLine(foo.ToLower()); //NullReferenceException!?!
    }
}

答えは、nullNullReferenceExceptionであるため、実際には a がスローされるということです。オブジェクトの基本コンストラクターは、独自のコンストラクターの前に呼び出されます。オブジェクトのコンストラクターで呼び出しを行うことにより、継承オブジェクトが完全に初期化される前にコードを実行する可能性が生じます。foovirtual

于 2008-09-23T07:17:47.547 に答える
166

C# のルールは、Java や C++ のルールとは大きく異なります。

C# でオブジェクトのコンストラクターを使用している場合、そのオブジェクトは、完全に派生した型として、完全に初期化された ("構築" されていない) 形式で存在します。

namespace Demo
{
    class A 
    {
      public A()
      {
        System.Console.WriteLine("This is a {0},", this.GetType());
      }
    }

    class B : A
    {      
    }

    // . . .

    B b = new B(); // Output: "This is a Demo.B"
}

これは、A のコンストラクターから仮想関数を呼び出すと、B の任意のオーバーライドが提供されていれば、それが解決されることを意味します。

システムの動作を十分に理解して意図的に A と B をこのように設定したとしても、後でショックを受ける可能性があります。B のコンストラクターで仮想関数を呼び出し、それらが B または A によって適切に処理されることを "知っている" とします。その後、時間が経ち、他の誰かが C を定義し、そこにある仮想関数のいくつかをオーバーライドする必要があると判断しました。突然、B のコンストラクターが C のコードを呼び出すことになり、非常に驚​​くべき動作につながる可能性があります。

C#、C++、および Java ではルールが大きく異なるため、コンストラクターで仮想関数を使用しないことをお勧めします。あなたのプログラマーは何を期待すべきかわからないかもしれません!

于 2008-09-23T07:36:08.687 に答える
90

警告の理由は既に説明されていますが、警告をどのように修正しますか? クラスまたは仮想メンバーのいずれかを封印する必要があります。

  class B
  {
    protected virtual void Foo() { }
  }

  class A : B
  {
    public A()
    {
      Foo(); // warning here
    }
  }

クラスAを封印できます：

  sealed class A : B
  {
    public A()
    {
      Foo(); // no warning
    }
  }

または、メソッド Foo をシールできます。

  class A : B
  {
    public A()
    {
      Foo(); // no warning
    }

    protected sealed override void Foo()
    {
      base.Foo();
    }
  }
于 2008-09-23T13:20:22.323 に答える
21

C# では、基本クラスのコンストラクターが派生クラスのコンストラクターの前に実行されるため、オーバーライドされる可能性のある仮想メンバーで派生クラスが使用するインスタンス フィールドはまだ初期化されていません。

これは、注意を払い、問題がないことを確認するための単なる警告であることに注意してください。このシナリオには実際の使用例があります。それを呼び出すコンストラクターが存在する場所の下の派生クラスで宣言されたインスタンス フィールドを使用できないという仮想メンバーの動作を文書化する必要があります。

于 2008-09-23T07:21:04.107 に答える
13

あなたがそれをしたくない理由については、上記のよく書かれた答えがあります。これは、おそらくそれを実行したい反例です ( Practical Object-Oriented Design in Ruby by Sandi Metz, p. 126から C# に翻訳されています)。

GetDependency()インスタンス変数に触れていないことに注意してください。静的メソッドが仮想である場合、それは静的になります。

(公平を期すために、おそらく依存性注入コンテナーまたはオブジェクト初期化子を介してこれを行うよりスマートな方法があります...)

public class MyClass
{
    private IDependency _myDependency;

    public MyClass(IDependency someValue = null)
    {
        _myDependency = someValue ?? GetDependency();
    }

    // If this were static, it could not be overridden
    // as static methods cannot be virtual in C#.
    protected virtual IDependency GetDependency() 
    {
        return new SomeDependency();
    }
}

public class MySubClass : MyClass
{
    protected override IDependency GetDependency()
    {
        return new SomeOtherDependency();
    }
}

public interface IDependency  { }
public class SomeDependency : IDependency { }
public class SomeOtherDependency : IDependency { }
于 2012-12-28T01:19:13.467 に答える
8

はい、コンストラクターで仮想メソッドを呼び出すのは一般的に悪いことです。

この時点では、オブジェクトはまだ完全に構築されていない可能性があり、メソッドによって期待される不変条件はまだ保持されていない可能性があります。

于 2008-09-23T07:15:32.027 に答える
6

欠けている重要なビットの 1 つは、この問題を解決する正しい方法は何ですか?

Greg が説明したように、ここでの根本的な問題は、派生クラスが構築される前に基底クラスのコンストラクターが仮想メンバーを呼び出すことです。

次のコードは、MSDN のコンストラクター デザイン ガイドラインから抜粋したもので、この問題を示しています。

public class BadBaseClass
{
    protected string state;

    public BadBaseClass()
    {
        this.state = "BadBaseClass";
        this.DisplayState();
    }

    public virtual void DisplayState()
    {
    }
}

public class DerivedFromBad : BadBaseClass
{
    public DerivedFromBad()
    {
        this.state = "DerivedFromBad";
    }

    public override void DisplayState()
    {   
        Console.WriteLine(this.state);
    }
}

の新しいインスタンスが作成されると、派生コンストラクターによってフィールドがまだ更新されていないためDerivedFromBad、基底クラスのコンストラクターが を呼び出してDisplayState表示します。BadBaseClass

public class Tester
{
    public static void Main()
    {
        var bad = new DerivedFromBad();
    }
}

改善された実装では、基本クラスのコンストラクターから仮想メソッドが削除され、Initializeメソッドが使用されます。の新しいインスタンスを作成するとDerivedFromBetter、予想される「DerivedFromBetter」が表示されます

public class BetterBaseClass
{
    protected string state;

    public BetterBaseClass()
    {
        this.state = "BetterBaseClass";
        this.Initialize();
    }

    public void Initialize()
    {
        this.DisplayState();
    }

    public virtual void DisplayState()
    {
    }
}

public class DerivedFromBetter : BetterBaseClass
{
    public DerivedFromBetter()
    {
        this.state = "DerivedFromBetter";
    }

    public override void DisplayState()
    {
        Console.WriteLine(this.state);
    }
}
于 2015-08-14T19:50:17.520 に答える
5

コンストラクターの実行が完了するまで、オブジェクトは完全にはインスタンス化されないためです。仮想関数によって参照されるメンバーは初期化されない場合があります。C++ では、コンストラクター内にいる場合、this作成中のオブジェクトの実際の動的型ではなく、現在のコンストラクターの静的型のみを参照します。これは、仮想関数の呼び出しが期待どおりに行われない可能性があることを意味します。

于 2008-09-23T07:14:10.877 に答える
5

コンストラクターは、仮想メソッドをオーバーライドするサブクラスのコンストラクターから (後でソフトウェアの拡張で) 呼び出される場合があります。サブクラスの関数の実装ではなく、基本クラスの実装が呼び出されます。したがって、ここで仮想関数を呼び出すことはあまり意味がありません。

ただし、設計がリスコフ置換の原則を満たしている場合、害はありません。おそらくそれが許容される理由です-エラーではなく警告です。

于 2008-09-23T07:25:01.973 に答える
5

他の回答がまだ対処されていないこの質問の重要な側面の 1 つは、基本クラスがコンストラクター内から仮想メンバーを呼び出すことが安全であることです。このような場合、派生クラスの設計者は、構築が完了する前に実行されるすべてのメソッドが、その状況下で可能な限り適切に動作することを保証する責任があります。たとえば、C++/CLI では、コンストラクターは、構築がDispose失敗した場合に部分的に構築されたオブジェクトを呼び出すコードでラップされます。このような場合の呼び出しDisposeは、リソース リークを防ぐために必要になることがよくありますが、Disposeメソッドが実行されるオブジェクトが完全に構築されていない可能性に備えて、メソッドを準備する必要があります。

于 2012-10-25T20:33:34.563 に答える
3

この警告は、仮想メンバーが派生クラスでオーバーライドされる可能性があることを示しています。その場合、親クラスが仮想メンバーに対して行ったことはすべて、子クラスをオーバーライドすることによって取り消されるか変更されます。明確にするために、小さな打撃の例を見てください

以下の親クラスは、コンストラクターの仮想メンバーに値を設定しようとします。これにより、Re-sharper 警告がトリガーされます。コードを見てみましょう。

public class Parent
{
    public virtual object Obj{get;set;}
    public Parent()
    {
        // Re-sharper warning: this is open to change from 
        // inheriting class overriding virtual member
        this.Obj = new Object();
    }
}

ここでの子クラスは、親プロパティをオーバーライドします。このプロパティが仮想としてマークされていない場合、コンパイラは、プロパティが親クラスのプロパティを非表示にすることを警告し、意図的な場合は「new」キーワードを追加することを提案します。

public class Child: Parent
{
    public Child():base()
    {
        this.Obj = "Something";
    }
    public override object Obj{get;set;}
}

最後に、使用への影響です。以下の例の出力では、親クラスのコンストラクターによって設定された初期値が破棄されます。 そして、これが Re-sharper が警告しようとしているものです。親クラス コンストラクターに設定された値は、親クラス コンストラクターの直後に呼び出される子クラス コンストラクターによって上書きされる可能性があります。

public class Program
{
    public static void Main()
    {
        var child = new Child();
        // anything that is done on parent virtual member is destroyed
        Console.WriteLine(child.Obj);
        // Output: "Something"
    }
}
于 2016-08-28T18:25:48.513 に答える
1

この特定のケースでは、C++ と C# の間に違いがあります。C++ では、オブジェクトは初期化されないため、コンストラクター内で仮想関数を呼び出すことは安全ではありません。C# では、クラス オブジェクトが作成されると、そのすべてのメンバーがゼロで初期化されます。コンストラクターで仮想関数を呼び出すことは可能ですが、まだゼロのメンバーにアクセスする可能性がある場合。メンバーにアクセスする必要がない場合は、C# で仮想関数を呼び出すのが非常に安全です。

于 2008-09-23T07:18:32.643 に答える
-3

私が見つけたもう1つの興味深いことは、ReSharperエラーは、私にはばかげている以下のようなことを行うことで「満足」できることです。ただし、以前に多くの人が述べたように、コンストラクターで仮想プロパティ/メソッドを呼び出すことはまだ良い考えではありません。

public class ConfigManager
{
   public virtual int MyPropOne { get; private set; }
   public virtual string MyPropTwo { get; private set; }

   public ConfigManager()
   {
    Setup();
   }

   private void Setup()
   {
    MyPropOne = 1;
    MyPropTwo = "test";
   }
}
于 2014-05-22T16:50:58.327 に答える