C#•2y ago

✅ Best practices regarding non-nullable property warnings?

Hi, In my Discord Bot project I get some warnings for example:

Non-nullable property 'Config' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.

I understand what it means and why I get it but not sure if it's ok for me to ignore it or if it indicates bad practices so I should learn to write it better.

Here is my Program.cs file:

namespace self_bot
{
    internal class Program
    {
        private static void Main(string[] args)
        {
            var bot = new Bot();
            bot.RunAsync().GetAwaiter().GetResult();
        }
    }
}

namespace self_bot
{
    internal class Program
    {
        private static void Main(string[] args)
        {
            var bot = new Bot();
            bot.RunAsync().GetAwaiter().GetResult();
        }
    }
}

namespace self_bot
{
    internal class Program
    {
        private static void Main(string[] args)
        {
            var bot = new Bot();
            bot.RunAsync().GetAwaiter().GetResult();
        }
    }
}

namespace self_bot
{
    internal class Program
    {
        private static void Main(string[] args)
        {
            var bot = new Bot();
            bot.RunAsync().GetAwaiter().GetResult();
        }
    }
}

And I've attached the Bot.cs file contents because it's too long to be put in this message.

The warning also shows with the Client and Slash properties.

From looking online the best way would either be to use lazy initialization or a constructor but I have no clue which is best for my project so would like advice from more experienced people.

Thanks

bot.txt5.05KB

IImmortal Hi, In my Discord Bot project I get some warnings for example: Non-nullable pro...

Jimmacle•5/2/24, 3:16 PM

you should never ignore them, it's common (at least in this echo chamber) to make them into compile errors instead

Jimmacle•5/2/24, 3:17 PM

is this a .NET Framework project? that version is not recommended anymore, the way your Program.cs is written makes me think that

JJimmacle is this a .NET Framework project? that version is not recommended anymore, the w...

ImmortalOP•5/2/24, 3:17 PM

.NET 8.0

Jimmacle•5/2/24, 3:18 PM

namespace self_bot;

internal class Program
{
    private static async Task Main(string[] args) => await new Bot().RunAsync();
}

namespace self_bot;

internal class Program
{
    private static async Task Main(string[] args) => await new Bot().RunAsync();
}

Jimmacle•5/2/24, 3:18 PM

you could make it even shorter if you use top level statements, then the whole file can just be

await new Bot().RunAsync();

await new Bot().RunAsync();

ImmortalOP•5/2/24, 3:19 PM

If I can make it that short would it just be better not have program.cs and bot.cs as separate files?

Jimmacle•5/2/24, 3:20 PM

i can't open the file you uploaded (CDN is blocked where i am) but probably

ImmortalOP•5/2/24, 3:24 PM

So like which approach should I go with to correct these warnings?

Jimmacle•5/2/24, 3:25 PM

you usually have 2 options

either you want the value to be nullable, so you add a ? to the type like string?string?
you don't want it to be nullable, in which case you make sure you initialize it with a non-null value

ImmortalOP•5/2/24, 3:26 PM

Yeah I was temporarily using ? to make it nullable but a Config for a bot should really be null I think so I removed it to learn better practice

becquerel•5/2/24, 5:45 PM

From looking online the best way would either be to use lazy initialization or a constructor but I have no clue which is best for my project so would like advice from more experienced people.

becquerel•5/2/24, 5:45 PM

Use a constructor

becquerel•5/2/24, 5:45 PM

Your dependencies in the Bot class (the config, the discord client) shouldn't be static unless there's a really good reason - instead have them injected into the class via the constructor

becquerel•5/2/24, 5:46 PM

the reason I say this is because your class's actual logic isn't static, and you should be highly cautious of mutable static fields/properties

becquerel•5/2/24, 5:47 PM

getting your dependencies via the constructor is also essentially the only way to be 100% sure they can be safely marked as non-null

becquerel•5/2/24, 5:49 PM

additionally, having a Bot construct its own config and connection to discord is too much work to have in one place - it violates the 'single responsibility principle' in technical speak. you could also look into the factory pattern if it only makes sense to construct your config/client as each Bot is created

becquerel•5/2/24, 5:49 PM

hope all that makes sense

Bbecquerel hope all that makes sense

ImmortalOP•5/2/24, 7:34 PM

Thank you, it makes sense. So I made a contructor to call the InitializeConfig method but that doesn't get rid of the warning. Can I not do that? bit off topic but should I be using a json to store the token? I also heard that storing the token as an environment variable is an option

https://paste.mod.gg/mogaxsqbjomh/0

BlazeBin - mogaxsqbjomh

A tool for sharing your source code with the world!

IImmortal Thank you, it makes sense. So I made a contructor to call the InitializeConfig m...

becquerel•5/2/24, 7:38 PM

it doesn't get rid of the warning because the nullability analyser is quite limited -- it can't 'see' inside of InitialiseConfig to realise it sets a value to your Config property

becquerel•5/2/24, 7:38 PM

whereas if you gave the property a value in the constructor directly, the warning would go away

becquerel•5/2/24, 7:38 PM

you can also add annotations to the InitializeConfig method to give the analyser some hints here, but that's a big rabbithole

ImmortalOP•5/2/24, 7:39 PM

Ahhh I see

becquerel•5/2/24, 7:39 PM

to clarify what i mean about doing stuff in the constructor, i would have it like this...

ImmortalOP•5/2/24, 7:40 PM

I think I got it

becquerel•5/2/24, 7:40 PM

public class Bot
{
public Config Config { get; private set; }
public DiscordClient Client { get; private set; }
public SlashCommandsExtension Slash { get; private set; }

public Bot(Config config, DiscordClient client)
{
Config = config;
Client = client;
}
}

ImmortalOP•5/2/24, 7:40 PM

https://paste.mod.gg/lrzeahwajrep/0

BlazeBin - lrzeahwajrep

A tool for sharing your source code with the world!

becquerel•5/2/24, 7:40 PM

ah, yes, that method works as well

ImmortalOP•5/2/24, 7:41 PM

Thanks for your help! :catlove:

becquerel•5/2/24, 7:42 PM

no prob!

Bbecquerel no prob!

ImmortalOP•5/2/24, 7:43 PM

also should I be using a json or environment variable for my token or does it not matter really?

becquerel•5/2/24, 7:44 PM

doesn't massively matter, both are miles better than hardcoding it into the app

becquerel•5/2/24, 7:45 PM

you might find it interesting to look into the 'options pattern' in .NET, represented with types like IOption<T> and IConfiguration

becquerel•5/2/24, 7:45 PM

it abstracts over 'read a json file' or 'read an environment variable' in a nice way

ImmortalOP•5/2/24, 7:45 PM

I'll look into it :>

ImmortalOP•5/2/24, 7:45 PM

Thank you so much c:

becquerel•5/2/24, 7:47 PM

https://tenor.com/view/type-computer-keyboard-cat-kitten-gif-12222055

Tenor

打字计算机电脑键盘猫小猫可爱

becquerel•5/2/24, 7:47 PM

enjoy

ImmortalOP•5/2/24, 7:47 PM

:catlove:

Angius•5/2/24, 10:14 PM

requiredrequired properties also do wonders

Angius•5/2/24, 10:15 PM

public class Bot
{
    public required Config Config { get; private set; }
    public required DiscordClient Client { get; private set; }
    public required SlashCommandsExtension Slash { get; private set; }
}

public class Bot
{
    public required Config Config { get; private set; }
    public required DiscordClient Client { get; private set; }
    public required SlashCommandsExtension Slash { get; private set; }
}

AAngius ```cs public class Bot { public required Config Config { get; private set; }...

ImmortalOP•5/3/24, 8:19 AM

Can't do that with private set since can't have access level be less visible than the class it's in so I'd have to remove private set from the properties

ImmortalOP•5/3/24, 8:20 AM

public class Bot
{
    public required Config Config { get; set; }
    public required DiscordClient Client { get; set; }
    public required SlashCommandsExtension Slash { get; set; }
}

public class Bot
{
    public required Config Config { get; set; }
    public required DiscordClient Client { get; set; }
    public required SlashCommandsExtension Slash { get; set; }
}

IImmortal Can't do that with private set since can't have access level be less visible tha...

Angius•5/3/24, 3:01 PM

Try an initinit instead of setset, then

Angius•5/3/24, 3:01 PM

It will still prevent the values from being overwritten

✅ Best practices regarding non-nullable property warnings?

Similar Threads

Similar Threads

Similar Threads