0

The following code throws a NRE exception trying to log the exception:

public static class FooClass
{
    public static string Foo { get; } = CalculateFoo();

    static CalculateFoo()
    {
        try
        {
            DoSomethingThatFails();
        }
        catch (Exception ex)
        {
           _log.Debug(ex); // ----> _log is null here <----
        }
    }

    static readonly ILog _log = LogManager.GetLogger("FooClass");
}

The reason _log is null, is because the compiler first initialize Foo property, and then it initializes the _log member.

A working solution is moving the _log initialization to the class top. This way the compiler first initializes _log and then Foo. However, we want to avoid that: In our organization, we have some coding rules that force us to sort code by visibility, so we first place public stuff at the class top, then, internal stuff, and finally private stuff.

Is there any way to tell the compiler the initialization order? If there is no way to do that, what would be, in your opinion, the best practices to fix this issue?

EDIT

I also tried to add a static constructor, but in that case the properties need to be calculated in the static constructor, just after initializing the _log.

Daniel Peñalba
  • 30,507
  • 32
  • 137
  • 219
  • 2
    Use a static constructor. – Johnathan Barclay Feb 14 '23 at 11:33
  • Don't use `_log`. Instead resolve a logger there and then OR [what Johnathan says](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/static-constructors) ^^. – Fildor Feb 14 '23 at 11:33
  • If you need a certain order do everything in the static constructor. – Ralf Feb 14 '23 at 11:39
  • What's wrong with calculating properties in the static constructor? Also because you have your own coding rules? – Sweeper Feb 14 '23 at 11:42
  • It's not wrong, just I was wondering if there is an attribute or something to tell the compiler, hey, compile this prop first. – Daniel Peñalba Feb 14 '23 at 11:49
  • Can you fix the errors in your code? Currently the operator `=>` after the property `Foo` is ambiguous, if it means `Foo { get => CalculateFoo() }`, `_log` should be initialized first. – shingo Feb 14 '23 at 12:02

2 Answers2

2

Instead of initialising things inline, you can initialise them in the static constructor, while considering their dependencies:

public static class FooClass
{

    // static constructors don't have a visibility, so your company's rules don't really apply here.
    // either way, you can put this anywhere in the class 
    static FooClass() {
        _log = LogManager.GetLogger("FooClass");
        Foo = CalculateFoo();
    }

    public static string Foo { get; }

    static string CalculateFoo()
    {
        ...
    }

    static readonly ILog _log;
    
}
Sweeper
  • 213,210
  • 22
  • 193
  • 313
1

I also tried to add a static constructor, but in that case the properties need to be calculated in the static constructor, just after initializing the _log.

You have to. There's three things at play here:

  1. A static constructor is called when its containing class is nonstatic and it is instantiated, or when any static member is referenced.
  2. Static members in C# are initialized in textual declaration order.
  3. Inline initializers (static Foo = Bar()) run before the static constructor.

The combination hereof causes your issue. So do as @Sweeper says: initialize all static members in a static constructor in an order you control. You can then adhere to your company's coding guidelines. Unless a new one pops up and proves them to be contradictory.

All of this can be avoided by not doing "SomethingThatFails" in a property getter. Getters should be quick and side-effect free. If they aren't, their logic should probably be in a method.

CodeCaster
  • 147,647
  • 23
  • 218
  • 272