Add helper for argument validation

[pascalberger]

Each Cake addin requires some kind of argument validation (or at least should :). Instead of having to implement this in every addin it would be nice if this could be provided for common cases (null check, empty string check, ...) by Cake.Core.

[pascalberger]

This could also be helpful Cake internally to simplify argument checking, like its eg done here.

If this is of interest I can provide a PR for a class similar to the one which we use in the Cake.Prca Addin.

[gep13]

@pascalberger I would have no objection to this.

@cake-build/cake-team thoughts?

[phillipsj]

This sounds pretty awesome if it makes it easier. It is a common pattern in every addin.

[daveaglick]

I agree, this would be great. There's already several argument validation frameworks - I wonder if any of them have source-only NuGet packages...

[agc93]

Agreed that it would be good to centralise this (few other candidates for that) 👍

@daveaglick I've used Ensure.That from @danielwertheim before, but looks like the source-only version hasn't gotten the latest updates yet..

[bjorkstromm]

Agreed!

But instead of signature void Check<T>(T...) I would rather see T Check<T>(T...). Then you could write checks like

_foo = Check.NotNull(foo);

I personally think ASP.NET team have a quite decent boilerplate here which I have used from time to time.

[agc93]

Additional reason to centralise this I just noticed: the Cake.Prca validation examples @pascalberger linked to currently show up on just about every type in the documentation, which could be a bit misleading, even with the From PrcaArgumentChecks)

[patriksvensson]

I like the link @mholo65 posted.

[pascalberger]

OK, I'll create a PR containing a class based on the ASP.NET boilerplate.

[patriksvensson]

@pascalberger Just make sure to attribute ASP.NET project in your code.

[pascalberger]

Since there's currently only interest for aliases providing argument checking but not also for a public class which can be consumed in addins, I'll leave this to someone else for implementing (or there is also already an implementation for such aliases in Cake.Incubator).

[bjorkstromm]

Adding Up-for-grabs on this one.

This is what we thought of:

• Add internal static class for argument checking. You could use this and attribute ASP.NET team in the comments. This file should be sourced in every project (<Compile Include="..\Shared\Check.cs" />)
• Change argument checking to use new approach. E.g. change this:

if (foo == null)
{
    throw new ArgumentNullException(nameof(foo));
}
_foo = foo;

to:

_foo = Check.NotNull(nameof(foo));

[patriksvensson]

@mholo65 Perhaps we should reserve this one for https://yourfirstpr.github.io/? I could do the mentoring.

[bjorkstromm]

@patriksvensson sounds good to me!

[bookman220]

@patriksvensson I'd be interested in trying this out

[patriksvensson]

@bookman220 Great! Do you want any help to get started or do you want to give it a shot yourself?

[bookman220]

@patriksvensson I'll probably need some help but let me figure out how to build this thing first.

[sarithsutha]

@patriksvensson @bookman220 I don't want to step on anyone's toes, I'm interested in trying this out if it is still up for grabs. This would be my first time contributing to open source.

[bookman220]

@sarithsutha @patriksvensson I do apologize for taking so long. I do think I may need some guidance however. Do I just open up the solution file in visual studio and compile it from there? If I haven't fixed the big within the next week you may take it sarithsutha

[sarithsutha]

@bookman220 thanks for your response. No rush, take your time, I don't mind waiting. I use Visual Studio 2017, I was able to just open the solution and compile it from there. And with VS code I was able to build using the Cake addin.

[bookman220]

@sarithsutha It appears I am having some trouble building the project. Apologies for needing such basic advice but I am new to Visual Studio and C#. It appears a fair amount of the solution failed to load and when I go to Build it just has the option to run a code analysis on the solution, which I did and you can see the output. Can you give me some pointers as to where to go from here?

[patriksvensson]

@bookman220 There seems to be problems loading the different projects according to the screenshot.

The following prerequisites need to be installed:

• Visual Studio 2017 (https://www.visualstudio.com/vs/)
• .NET Core SDK 1.0.4 or later (https://www.microsoft.com/net/download/core)

The build.cake file is only used to build the project from command line. When the project is loading correctly in Visual Studio, you should be able to build everything from the Build menu.

[micheleissa]

@patriksvensson I'd like to take a try on this one.

[micheleissa]

What I'm planning to do :

• Add folder called 'Shared' in the solution directory.
• Add 'Check.cs' inside that folder.
• Modify each csproj file to include

<ItemGroup>
    <Compile Include="..\Shared\Check.cs" />
</ItemGroup>

• The namespace for it Cake.Shared
• I saw the link from ASP.NET team but I feel it is more complicated than it should be and I was thinking of something along the following

public static T NotNull<T>(T o, string message = null)
            {
            if (o == null)
                {
                throw new ArgumentNullException(message ?? $"{nameof(o)} cannot be null");
                }
            return o;
            }

Thoughts?

[kcamp]

As a developer, it would be preferable to more correctly support parameter names; nameof(o) is not going to be semantically useful in the exception that's thrown.

Reference: https://msdn.microsoft.com/en-us/library/k8a0dfcy(v=vs.110).aspx

Consider something like this, or overloads that would support the same..

T NotNull<T>(T item, string parameterName, string message = null)  { .... }

So that the calling convention would be more akin to

public SomeConstructor(object arg) 
{
    _fieldName = ValidationHelperClassName.NotNull(arg, nameof(arg));
    ....

[patriksvensson]

@micheleissa This is still assigned to @bookman220. @bookman220 Are you still working on this?

[micheleissa]

@kcamp What would you use parameterName for ? in the case someone passes 'bogus' as the parameterName and it is used in the exception message the result will not be accurate in my opinion.

[patriksvensson]

@micheleissa What @kcamp means is that nameof(o) will always print o. It will never used the original parameter name.

[micheleissa]

@patriksvensson gotcha. Silly me!

[micheleissa]

So are you supposed to wait for a response from 'bookman220'