10

I have some non-nullable fields which I want to initialize inside a helper methods, called from a constructor, to reduce the clutter inside the constructor:

private FlowLayoutPanel _flowPanel;
private ComboBox _printersComboBox;
//...

public PrintSettingsView(IServiceProvider serviceProvider, IPrintSettings printSettings)
{
    InitializeComponent();
    PostInitializeComponent(); // this is where _flowPanel etc get initialized
    // ...
}

How do I avoid warnings like Non-nullable field '_flowPanel' must contain a non-null value when exiting constructor. Consider declaring the field as nullable?

The best idea I've come up with so far:

public static void Assert([DoesNotReturnIf(false)] bool condition)
{
  if (!condition) throw new InvalidOperationException();
}

public PrintSettingsView(IServiceProvider serviceProvider, IPrintSettings printSettings)
{
    InitializeComponent();
    PostInitializeComponent();

    Assert(_flowPanel != null);
    Assert(_printersComboBox != null);
    //...
}

It's still getting messy when there's a lot of fields. Is there anything better than this?

It's a .NET 6 project, so I could use the latest and the greatest.

enter image description here

noseratio
  • 59,932
  • 34
  • 208
  • 486
  • 6
    No good solutions. The alternative is to declare the fields `private FlowLayoutPanel _flowPanel = null!;` to shut the analyser up, and then trust yourself to correctly initialise it – canton7 Oct 28 '21 at 09:24
  • @canton7 thanks, I haven't thought of `= null!` actually... not sure I like it more than `Assert` though :) – noseratio Oct 28 '21 at 09:30
  • 2
    The problem is static analysis and the analyzer cannot and does not attempt to follow complex path to work out if you know what you are doing. You either need to pragma it, set it to default! or null! or write some sort of weird superfluous method. – TheGeneral Oct 28 '21 at 09:30
  • 2
    `[MemberNotNullWhen(true, nameof(_flowPanel)), MemberNotNullWhen(...)] bool PostInitializeComponent() { ...; return true; }`. Then in the constructor `Debug.Assert(PostInitializeComponent())`. The drawback is that you must spell out every member this way, which could get tedious. – Jeroen Mostert Oct 28 '21 at 09:41
  • 3
    @JeroenMostert by the way there exists just `MemberNotNull(nameof(...))`. – Evk Oct 28 '21 at 11:01
  • @Evk: I thought there might, I was too lazy to search for it after remembering `MemberNotNullWhen` (because it's, um, cooler). – Jeroen Mostert Oct 28 '21 at 11:06
  • If you do not reassign them anywhere (quite likely in this case) - argument can be made that they should be readonly, in which case you still need to assign them from inside constructor. So I'd think how to do just that instead of trying to make analyzer work. – Evk Oct 28 '21 at 11:26
  • @Evk a good point, and this is similar to what @DamienG suggested, but it looks like I either have to have a huge tuple to return, or a long list of `out` arguments. – noseratio Oct 28 '21 at 11:36
  • 1
    Or you can live with cluttered constructor. Or maybe move each field initialization into its own separate method (`InitPrintersComboBox` etc). Tuple \ out parameters are too easy to mess up (assign value to the wrong field). – Evk Oct 28 '21 at 11:40
  • @Evk, one other option might be to use a helper private constructor: `public PrintSettingsView(IServiceProvider serviceProvider, IPrintSettings printSettings): this(/* init the UI */)` in place of `PostInitializeComponent`. – noseratio Oct 29 '21 at 01:47

4 Answers4

10

You have a few options:

  1. Use [MemberNotNull] on the helper methods. This is a bit verbose but it should work. See https://stackoverflow.com/a/64958374/2855742.
  2. Return values from your helper methods (maybe tuples?) and deconstruct-assign right into the fields you want to initialize.
Rikki Gibson
  • 4,136
  • 23
  • 34
8

Another option is to declare the fields as:

private FlowLayoutPanel _flowPanel = null!;
private ComboBox _printersComboBox = null!;

This forces the compiler to consider these fields as initialized, and the ! suppresses warnings that the value is null yet the field is not nullable.

Note that adding the null assignment does not change the generated code. There is no run-time impact, only design-time.

Drew Noakes
  • 300,895
  • 165
  • 679
  • 742
2

Could you have PostInitializeComponent return the FlowPanel?

then

public PrintSettingsView(IServiceProvider serviceProvider, IPrintSettings printSettings)
{
    InitializeComponent();
   _flowPanel = PostInitializeComponent();
    // ...
}

If PostInitializeComponent does a bunch of work maybe extract out the part that builds FlowPanel and have that return it and assigned.

DamienG
  • 6,575
  • 27
  • 43
  • The only problem is, I have like a dozen of those fields (like on the pic in the q), but maybe I can return a tuple of multiple values and deconstruct it? Thanks for this idea! – noseratio Oct 28 '21 at 10:09
0

what about defining them as nullable like:

FlowLayoutPanel? _flowPanel;

  • 1
    I want to keep them non-nullable throughout the rest of the code, because that's what they are as once I leave the constructor. – noseratio Oct 28 '21 at 09:39