Dealing with non-nullable warning in constructor chain

user_5525302 0
Viewed 378

How to deal with the "Non-nullable property 'S2' must contain a non-null value"-warning for the private constructor in the following example:

 #nullable enable
 class X {...}
 class Y {...}
 class Z {...}     
 class C {
   public string S1 { get; }
   public string S2 { get; }
   private C(Z z) => S1 = GenerateS1FromZ(z);
   public C(Z z, X x) : this(z) => S2 = GenerateS2FromX(x);
   public C(Z z, Y y) : this(z) => S2 = GenerateS2FromY(y);

   private static string GenerateS1FromZ(Z z) { ... }
   private static string GenerateS2FromX(X x) { ... }
   private static string GenerateS2FromY(Y y) { ... }
 }

S1 must be initialized in a constructor, since it has no setter. S2 can be initialized in two ways, so there are two public constructors.

Assume that S1 and S2 are just placeholders for a few more properties. Therefore, it is not practical to pass the value for S2 to the private constructor as parameter or to initialize S1 in both public constructors.

Under this assumption the code above is ok I think, but the non-nullable-warning does not work as I wish. One could add S2=null!; in the private constructor to avoid the warning, but that seems to be a hack.

c#
2 Answers

Referring to your requirement

Assume that S1 and S2 are just placeholders for a few more properties. Therefore, it is not practical to pass the value for S2 to the private constructor as parameter or to initialize S1 in both public constructors.

This is a good hint to do some refactoring and extract those properties to separate classes or structs. If you do so, you can then pass the values to your private constructor of C and remove the non-nullable warning.

Here is a simple example for completeness

class S1
{
    public string S1Prop { get; }

    public S1(Z z) => S1Prop = GenerateFrom(z);
    private static string GenerateFrom(Z z) { /* ... */ return "From z"; }
}

class S2
{
    public string S2Prop { get; }

    public S2(X x) => S2Prop = GenerateFrom(x);
    public S2(Y y) => S2Prop = GenerateFrom(y);
    private static string GenerateFrom(X x) { /* ... */ return "From x"; }
    private static string GenerateFrom(Y y) { /* ... */ return "From y"; }
}

class C
{
    public S1 S1 { get; }
    public S2 S2 { get; }

    private C(S1 s1, S2 s2) { S1 = s1; S2 = s2; }

    public C(Z z, X x) : this(new S1(z), new S2(x)) { }
    public C(Z z, Y y) : this(new S1(z), new S2(y)) { }
}

Depending on your requirements you may also remove the dependency on the classes X, Y and Z by making the private constructor public and remove the other two.

class C2 // No dependency to X, Y, Z
{
    public S1 S1 { get; }
    public S2 S2 { get; }

    public C2(S1 s1, S2 s2) { S1 = s1; S2 = s2; }
}
user_93934580

Although assigning S2 to null! may stop compiler from complaining, it makes code more fragile to further modification because someone else might add new constructor without initializing S2.

In my opinion best solution is to create constructor that will assign S2 and call it everywhere.

#nullable enable
    class X { }
    class Y { }
    class Z { }
    class C
    {
        public string S1 { get; }
        public string S2 { get; }
        private C(Z z, string s2)
        {
            S1 = GenerateS1FromZ(z);
            S2 = s2; // assign here
        }

        public C(Z z, X x) : this(z, GenerateS2FromX(x)) { } // remove assignment here
        public C(Z z, Y y) : this(z, GenerateS2FromY(y)) { }

        private static string GenerateS1FromZ(Z z) => "";
        private static string GenerateS2FromX(X x) => "";
        private static string GenerateS2FromY(Y y) => "";
    }
user_103396750
Related