C#•3mo 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•3mo 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

Immortal•3mo ago

.NET 8.0

Jimmacle•3mo 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();

Immortal•3mo ago

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

Jimmacle•3mo ago

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

Immortal•3mo ago

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

Jimmacle•3mo 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

Immortal•3mo 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•3mo 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

Immortal•3mo 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•3mo 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

Immortal•3mo ago

Ahhh I see

Becquerel•3mo ago

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

Immortal•3mo ago

I think I got it

Becquerel•3mo 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; } }

Immortal•3mo ago

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

BlazeBin - lrzeahwajrep

A tool for sharing your source code with the world!

Becquerel•3mo ago

ah, yes, that method works as well 🙂

Immortal•3mo ago

Thanks for your help! :catlove:

Becquerel•3mo ago

no prob!

Immortal•3mo ago

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

Becquerel•3mo 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

Immortal•3mo ago

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

Becquerel•3mo ago

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

Tenor

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

Becquerel•3mo ago

enjoy

Immortal•3mo ago

:catlove:

Angius•3mo 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; }
}

Immortal•3mo 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•2mo 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?