add dictionary `.IfContainsKey()` `.IfNotContainsKey()`

[amantinband]

Add the following extensions methods for dictionaries:

var dictionary = new Dictionary<string, string>
{
    ["key1"] = "value1",
    ["key2"] = "value2",
};

dictionary.Throw().IfContainsKey("key1");
dictionary.Throw().IfNotContainsKey("key3");

And the corresponding dictionary properties extension methods:

var dictionary = new Dictionary<string, string>
{
    ["key1"] = "value1",
    ["key2"] = "value2",
};

var foo = new { Values = dictionary };

foo.Throw().IfContainsKey(f => f.Values, "key1");
foo.Throw().IfNotContainsKey(f => f.Values, "key3");

We don't have dictionary validation yet, so this will hopefully be the start of a bigger feature 🚀

[beton-kun]

I want to develop this issue

[AndreasHoersken]

Oh boy - I'm sorry @beton-kun I did not see your first comment there today.

[AndreasHoersken]

I closed my first PR as the code only works with IDictionary-interface and not with the IDictionary<,> generic interface. The sample given in #40 works as the generic class Dictionary<,> implements the non-generic interface.

@beton-kun : Your comment was right, where is it? As an alternative to another generic parameter perhaps we could have two extension-methods (one for IDictionary, one for IDictionary<,>) or infer the generic arguments with reflection somehow? Any thoughts?

[beton-kun]

@AndreasHoersken listen, gtfo from my issue

[AndreasHoersken]

@beton-kun are you serious? No collaboration here? Have fun then. Looking forward to your solution.

[amantinband]

@AndreasHoersken listen, gtfo from my issue

+

=
I've blocked @beton-kun.

This is a library from the community for the community. No bullying or disrespectful comments will be tolerated.

@AndreasHoersken, let me poke at this a little and we'll collaborate on a solution for this?

[AndreasHoersken]

Wow. Strange guy. Thanks @mantinband . Sorry for the inconvenience. I'll be happy to work on it further. If you have any ideas on the solution, please let me know. I can only work on it in the evenings though.

[amantinband]

@AndreasHoersken I see the issue you ran into. I had a similar issue with #20.

There is a way to work around this, I prototyped it in the following PR: #25

Basically what we would have to do is:

1. Update the Validatable struct to inherit from some interface which defines TValue as covariant

public readonly record struct Validatable<TValue>(TValue Value, string ParamName, ExceptionCustomizations? ExceptionCustomizations = null)
    : IValidatable<TValue> {}

public interface IValidatable<out TValue> {}

2. Recieve the validatable as the interface. something like this:

    public static IValidatable<IDictionary<TKey, TValue>> IfContainsKey<TKey, TValue>(this IValidatable<IDictionary<TKey, TValue>> validatable, TKey key)
    {
        Console.WriteLine($"Contains key: {validatable.Value.ContainsKey(key)}");
        return validatable;
    }

3. Then, all the following are supported:

IDictionary<string, string> dictionary1 = new Dictionary<string, string>
{
    { "key", "value" },
    { "key2", "value2" },
};

Dictionary<string, string> dictionary2 = new()
{
    { "key", "value" },
    { "key2", "value2" },
};

var dictionary3 = new ReadOnlyDictionary<string, string>(dictionary1);
var dictionary4 = new ConcurrentDictionary<string, string>(dictionary1);
// etc

The only issue with this solution is that when passing the Validatable struct to the method, it will be boxed, which will have runtime consequences..
It also means we cannot use this method interchangeably with the extension methods that receive a Validatable, i.e this won't work:

dictionary.Throw().IfContainsKey("key").If{other-methods}(dictionary2);

Unless we change all places to accept an IValidatable and then we have the issue that the Validatable will always be boxed, which again, isn't great.

I wonder if there is some way around the boxing, or maybe a different solution I'm missing. What do you think?

[AndreasHoersken]

@mantinband - Thank you for your deliberate analysis!

I tried out two things myself

public static ref readonly Validatable<TValue> IfContainsKey<TValue,TDictKey,TDictValue>(this in Validatable<TValue> validatable, TDictKey dictKey)
        where TValue : IDictionary<TDictKey,TDictValue>
        where TDictKey : notnull

and

    public static ref readonly Validatable<IDictionary<TDictKey, TDictValue>> IfContainsKey<TDictKey, TDictValue>(this in Validatable<IDictionary<TDictKey, TDictValue>> validatable, TDictKey dictKey)
        where TDictKey : notnull
    {
        Validator.ThrowIfContainsKey(validatable.Value, dictKey, validatable.ParamName, validatable.ExceptionCustomizations);

        return ref validatable;
    }

Both do not work because the missing covariant interface. What I do not get is why the implementation for collections does work, as it uses the same strategy:

[MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static ref readonly Validatable<TValue> IfContains<TValue, TElement>(
        this in Validatable<TValue> validatable,
        TElement element)
        where TValue : notnull, IEnumerable<TElement?>
        where TElement : notnull
    {
        Validator.ThrowIfContainsElement(
            validatable.Value,
            element,
            validatable.ParamName,
            validatable.ExceptionCustomizations);

        return ref validatable;
    }

Even with subclasses of IEnumerable, i.e. List. Why isn't the missing covariant interface a problem here?

So after all, I think we need the covariant interface. Regarding your points:

1. Boxing / Unboxing. It is not ideal as we have performance & memory overhead, but I do not think it is to bad in the context of error-handling either.
   Why is Validatable implemented as record struct after all? If it is performance, I personally would choose records or a simple classes over the boxing / unboxing - penalties we get. A little side effect here would be a better backportability to older .net-Versions.

2. Regarding the changes: If we switched to the interface, I would do it for the whole solution to preserve the possibility of method-chaining. One reason more against the record structs.

@mantinband: What do you think? Is there another reason for using record structs beside performance benefits?

[amantinband]

@AndreasHoersken first of all thanks for taking the time to think about these issues, I'm happy to have you working with me on this project 🙏🏽

1. How does it work with collections - Excellent question, actually 🤓😂 It was a lot of trial and error solving the similar collection issue.
2. class vs struct: The reason behind using a struct is performance. I think for many projects, the decision on which validation library to use comes down to what has the least performance overhead.
   From the anecdotal benchmarks I ran a few weeks ago comparing Throw against the most common validation libraries, we were considerably faster than some and not slower than any. Following your reply I ran some benchmarks comparing using struct vs using a class & accepting it via the covariant interface 👇🏽:
   
   struct not only has no GC overhead, it is also twice as fast.
   (I create a gist if you want to play around with it: https://gist.github.com/mantinband/f86084c72a414dbafa784a2b66033e6e)
3. backward compatibility: My main motivation to create Throw was to produce a meaningful error response (String should not be longer than 3 characters. (Parameter 'user: u => u.Name') without having to pass any nameofs (nameof(user)) or messages (user: u => u.Name). This wasn't possible before .NET 6 introduced the CallerArgumentExpression, so unfortunately this library can't be backward compatible.

What do you think?

[AndreasHoersken]

@mantinband: I think I can learn a lot here, so its my pleasure :-). Thanks again for taking your time to explain things :-).

Ad 1.) I would have hoped that, if we understand the "simple case" with one generic param in collections, we could get a simple solution here... I will have another look on that over the weekend. Perhaps this one works because IEnumerable<> is itself covariant. That would not apply to IDictionary<>.

Ad 2.) Benchmark:
a) In your Benchmark, no boxing / unboxing does occur, does it? This would be interesting for our decision how to proceed on this issue. I will add this to your gist and share the results over the weekend.
b) Even if the record struct is twice as fast as the record class in this (simple) benchmark, I'm asking myself if it would matter in a real world scenario. So if I think of a real world case in a cloud environment with much communication going over the network, I don't belief that 2 ns is a critical point here. Same holds for 99.9% of projects, I think.
If, on the other side, we have a performance-critical usecase, I would think that you would not use validation-methods, where i.e. whole collections are enumerated to get the count (if I could get it with a list.length otherwise). I even would argue that you would omit exception-handling all together and move it to a non performance-critical place, if possible.

All in all, it feels a little bit like a case of premature optimization. But then again, I don't know the domain.
Perhaps #35 would be a good starter to go into this topic a little further.

Ad 3.) Thats interesting. I did not have the CallerArgumentExpression in mind.

---

Edit: The statement regarding enumerating the whole collections ist not correct. I did not see the pattern matching expression in GetCollectionCount, but the pattern matching also comes to a price (see next comment).