Throw when xxxOptions are null

[nulltoken]

From https://github.com/libgit2/libgit2sharp/pull/1068/files#r31650592

We rely on the following pattern throughout the codebase.

public Things DoThings()
{
     return DoThings(null);
}

public Things DoThings(ThingsOptions options)
{
     options = options ?? new ThingsOptions();
     [...]
}

Let's change this to the following one

public Things DoThings()
{
     return DoThings(new ThingsOptions());
}

public Things DoThings(ThingsOptions options)
{
     Ensure.ArgumentNotNull(options);
     [...]
}

[Therzok]

This would introduce API contract breakage (huge one) and code verbosity on user side. I for one am against this change.

[nulltoken]

The table below represents the impact of the proposed change.

Code	Current Impl.	Proposed Impl.
DoThings();	✅	✅
DoThings(new Options());	✅	✅
DoThings(null);	✅	💥

I agree about the API code contract breakage. However, now is the time for this (hence the stabilization label). v0.22 is taking quite a long time to get out of the woods as we're trying to obsolete as many things as possible in order to be able to release v1.0 as the next version after it.

Regarding the additional code verbosity, I'm not sure to see what you refers to. Could you please elaborate further on this topic? Maybe with an example?

From another standpoint, I'm not sure I see the added value of allowing DoThings(null) code construct.

[Therzok]

Ah, so, basically, you wouldn't be able to pass null, but the overload will construct new default options. I get it, and it works.

Probably all the overloads should go into extensions anyway.

[Therzok]

Aka:

public Things DoThings()
{
     return DoThings(new ThingsOptions());
}

Should be in a:

public static class ThingsExtensions
{
    public Things DoThings(this ThingClass things)
    {
        return new things.DoThings(new ThingsOptions());
    }
}

[nulltoken]

Yes. That was the idea 😉

[whoisj]

👍