C#•7mo 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();
        }
    }
}

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.txt

28 Replies

Jimmacle•7mo ago

you should never ignore them, it's common (at least in this echo chamber) to make them into compile errors instead is this a .NET Framework project? that version is not recommended anymore, the way your Program.cs is written makes me think that

ImmortalOP•7mo ago

.NET 8.0

Jimmacle•7mo ago

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();
}

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•7mo ago

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

Jimmacle•7mo ago

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

ImmortalOP•7mo ago

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

Jimmacle•7mo ago

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

ImmortalOP•7mo ago

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•7mo ago

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.

Use a constructor 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 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 getting your dependencies via the constructor is also essentially the only way to be 100% sure they can be safely marked as non-null 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 hope all that makes sense

ImmortalOP•7mo ago

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!

Becquerel•7mo ago

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 whereas if you gave the property a value in the constructor directly, the warning would go away you can also add annotations to the InitializeConfig method to give the analyser some hints here, but that's a big rabbithole

ImmortalOP•7mo ago

Ahhh I see

Becquerel•7mo ago

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

ImmortalOP•7mo ago

I think I got it

Becquerel•7mo ago

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•7mo ago

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

BlazeBin - lrzeahwajrep

A tool for sharing your source code with the world!

Becquerel•7mo ago

ah, yes, that method works as well 🙂

ImmortalOP•7mo ago

Thanks for your help! :catlove:

Becquerel•7mo ago

no prob!

ImmortalOP•7mo ago

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

Becquerel•7mo ago

doesn't massively matter, both are miles better than hardcoding it into the app you might find it interesting to look into the 'options pattern' in .NET, represented with types like IOption<T> and IConfiguration it abstracts over 'read a json file' or 'read an environment variable' in a nice way

ImmortalOP•7mo ago

I'll look into it :> Thank you so much c:

Becquerel•7mo ago

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

Tenor

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

Becquerel•7mo ago

enjoy

ImmortalOP•7mo ago

:catlove:

Angius•7mo ago

required properties also do wonders

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; }
}

ImmortalOP•7mo ago

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

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; }
}

Angius•7mo ago

Try an init instead of set, then It will still prevent the values from being overwritten

Gaming

Programming

✅ Best practices regarding non-nullable property warnings?