8
\$\begingroup\$

I've no idea what to call this, so I called it Classify since that's how I use it.

Basically, the idea is to pass a Dictionary<TResult, Func<TValue, bool>> which is evaluated for an input, and the first Func<TValue, bool> returning true will return the TResult key.

The intention is to make this a generic extension method.

public TResult Classify<TValue, TResult>(TValue input, Dictionary<TResult, Func<TValue, bool>> classifications)
{
    foreach (var classification in classifications)
    {
        if (classification.Value.Invoke(input))
        {
            return classification.Key;
        }
    }

    return default(TResult);
}

Pretty simple, but pretty useful (for me at least) as well.

Usage:

<td class='<%#Classify(((ApiBatchLog)Container.DataItem).RequestsPerSecond, new Dictionary<string, Func<double, bool>> { ["good"] = x => x >= 3, ["neutral"] = x => x > 2, ["bad"] = x => true }) %>'>
    <%#((ApiBatchLog)Container.DataItem).RequestsPerSecond.ToString("0.00") %>
</td>
\$\endgroup\$
  • 2
    \$\begingroup\$ Why not use a ternary expression like x >= 3 ? "good" : x > 2 ? "neutral" : "bad" instead? It's more succinct, faster and even more readable in my opinion (having the condition in front of the result looks more natural to me, and there are no = x => in there that make the actual conditions harder to spot). \$\endgroup\$ – Pieter Witvoet Aug 19 '16 at 20:21
  • 1
    \$\begingroup\$ Way too much code mixed into your aspx if you ask me. \$\endgroup\$ – RubberDuck Aug 20 '16 at 1:23
  • \$\begingroup\$ @PieterWitvoet Because x doesn't exist on the front end, so I have to case twice to make it work: ((ApiBatchLog)Container.DataItem).RequestsPerSecond >= 3 ? "good" : ((ApiBatchLog)Container.DataItem).RequestsPerSecond >= 2 ? "neutral" : "bad". I want to avoid the double-casting (or more, depending on thresholds and conditions). \$\endgroup\$ – 410_Gone Aug 20 '16 at 5:13
  • \$\begingroup\$ In that case, what about TOutput Classify<TInput, TOutput>(TInput input, Func<TInput, TOutput> translate), which can be called like Classify(((ApiBatchLog)Container.DataItem).RequestsPerSecond, (x) => x >= 3 ? "good" : x >= 2 ? "neutral" : "bad")? \$\endgroup\$ – Pieter Witvoet Aug 20 '16 at 10:14
  • \$\begingroup\$ Dictionary is not ordered. You can achieve the same syntax with a custom Add method and a custom class. \$\endgroup\$ – usr Aug 20 '16 at 18:32
8
\$\begingroup\$

Three small things:

  1. A Func<TValue, bool> is more commonly known as a Predicate<TValue>
  2. You're mapping ... the "wrong" way round. You're mapping a predicate-match to a value. Your dictionary should reflect that. I strongly suggest you use a Dictionary<Predicate<TValue>, TResult> instead to be more "semantically correct" (or at least semantically predictable)
  3. Your Dictionary iteration is not guaranteed to always yield the same results as is. Dictionary doesn't generally have a guaranteed iteration order. Instead you should consider using a SortedDictionary
\$\endgroup\$
  • 1
    \$\begingroup\$ The first makes complete sense, the second does as well but isn't what I want (I like the string => Predicate<T> mapping), and the third should (I would think) be fixed by LINQ FirstOrDefault mentioned in Mat's Mug's answer. \$\endgroup\$ – 410_Gone Aug 20 '16 at 5:22
  • 2
    \$\begingroup\$ @EBrown: FirstOrDefault depends on the order of the collection, so using it won't fix issue #3. \$\endgroup\$ – Pieter Witvoet Aug 20 '16 at 10:28
8
\$\begingroup\$

Needs more LINQ:

return classifications.FirstOrDefault(item => item.Value.Invoke(input)).Key;

This has the advantage (IMO) of highlighting the fact that you're discarding all but the first match (note that iteration order is not guaranteed!).

Classify doesn't really convey the fact that you're returning a Key from a dictionary. Seems FindKey would be a much better name.

A generic extension method seems like a good idea - call your source source, and stick to TKey and TValue:

public static TKey FindKey<TKey, TValue>(this IDictionary<TKey, Func<TValue, bool>> source, TValue input)
{
    return source.FirstOrDefault(item => item.Value.Invoke(input)).Key;
}

That one-liner makes use of the fact that KeyValuePair<TKey,TValue> is a struct (and therefore can't be null), so you can just return .Key and that will be default(TKey).

\$\endgroup\$
  • \$\begingroup\$ In this context probably GetClass would be even better. \$\endgroup\$ – t3chb0t Aug 19 '16 at 19:43
  • \$\begingroup\$ @t3chb0t I'm making it an all-access generic method. \$\endgroup\$ – 410_Gone Aug 19 '16 at 19:47
  • \$\begingroup\$ You are calling FirstOrDefault, but forgetting to set the default to default(T), and therefore have the potential to throw an NRE when you try to access Key. \$\endgroup\$ – Hosch250 Aug 19 '16 at 21:28
  • \$\begingroup\$ @Hosch250 T is a KeyValuePair<TKey,TValue> which is a value type and therefore can't be null; assuming the Func delegate won't throw, this code couldn't throw a NRE if it tried. \$\endgroup\$ – Mathieu Guindon Aug 19 '16 at 22:09
  • 1
    \$\begingroup\$ Oh, OK. Would you like me to try that? Set classifications to null for starters. \$\endgroup\$ – Hosch250 Aug 19 '16 at 22:35
5
\$\begingroup\$

I'm not sure about the design. It's not really testable, reusable and maintainable. I could imagine that something like this could be more reliable:

interface IEvaluator<TResult, TValue>
{   
    TResult Evaluate(TValue value);
}

class RequestsPerSecondEvaluator : IEvaluator<string, double>
{
    public string Evaluate(double value)
    {
        if (value >= 3) return "good";
        if (value > 2) return "neutral";
        return "bad";
    }
}

and

<td class='<%#new RequestsPerSecondEvaluator().Evaluate(((ApiBatchLog)Container.DataItem).RequestsPerSecond)) 

Especially I'm not a fan of any business-logic in the markup.

\$\endgroup\$
  • \$\begingroup\$ This is literally the opposite of what I want. I want the conditions on the markup side, so that if I change conditions I don't have to redeploy the entire binary. \$\endgroup\$ – 410_Gone Aug 20 '16 at 5:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.