Question: how would nullable flow analysis behave for method accessing non-nullable field called by constructor

[huoyaoyuan]

Suppose this code:

class C
{
    readonly Something something;
    public C(Parameter parameter)
    {
        // if Update() is called here, null reference will accur
        something = new Something(parameter);
        something.SomeEvent += Update;
        Update(); // Use this pattern to show that calling method in constructor is sometimes necessary.
    }

    void Update() => this.SomeChild = something.SomeChild;
}

Have flow analysis in this scenario been discussed?
In my opinion, calling method before assigning all non-nullable field should give a warning.

[yaakov-h]

That's totally fine, no problemo.

The field something is declared as Something, which is non-nullable. The field is set before Update() is called, so Update() won't crash.

As far as Update() is concerned, it is accessing a member of a non-null reference, which is OK.

At the moment though, if you call Update() before setting something, there is no compiler warning and the program will crash. I don't know if this is something that the language/compiler team will address or not.

[theunrepentantgeek]

In my opinion, calling method before assigning all non-nullable field should give a warning.

I disagree.

This would preclude the use of factory methods to create the value to store in the field. Such methods can't always be static because sometimes they need to be overridable - and it's a useful design technique.

[Joe4evr]

and it's a useful design technique

Despite it being a pretty big anti-pattern?

[huoyaoyuan]

@theunrepentantgeek Factory method and consuming method are two different scenarios, and should be covered by seperated rules.
Currently, factory method cannot set readonly fields, which has been requested.

[yaakov-h]

See also: CA2214.

[jnm2]

@Joe4evr

Despite it being a pretty big anti-pattern?

Used judiciously, it solves problems that aren't otherwise solvable.

[theunrepentantgeek]

@huoyaoyuan I was thinking about a private factory method called by the constructor:

public abstract class FooBase
{
    private readonly IChannel _channel;

    public FooBase()
    {
        _channel = CreateChannel();
    }

    protected abstract IChannel CreateChannel();
}

public class FooActual : FooBase
{
    public FooActual() : base()
    {
    }

    protected override IChannel CreateChannel()
    {
        return new ActualChannel();
    }
}

@Joe4evr I agree that it can be misused - but whether it's an antipattern or not is up for debate.

I've worked with C++ developers who were repulsed by it (no exaggeration) and who went to great lengths to avoid calling virtual methods from constructors - including the abomination of "Consumers need to always call Initialize() after construction." (With the rise of RAII, this has fortunately mostly gone away.)

In the Smalltalk community, as I understand it, using factory methods so that descendant classes can change the defaults is idiomatic and normal.

@jnm2 I've primarily seen this in cases where a large class hierarchy has evolved over time, and it proved pragmatic to use a virtual factory method to accommodate changing requirements in a way that didn't require wholesale modification of every leaf class.

[jnm2]

Yes, I've been extremely glad for it at times.