Finding MinMax values and returning a tuple as a result

Posted on

Problem

I was coding some tooling stuff when I started to ponder whether the best return type for a Range / MinMax method applied on a IEnumerable method.

I am using the value the C# 7 Tuples like below, but tempted to create my own class with readonly fields.

    public static (TSource Min, TSource Max) MinMax<TSource>(this IEnumerable<TSource> source, IComparer<TSource> comparer)
    {
        using (var sourceIterator = source.GetEnumerator())
        {
            if (!sourceIterator.MoveNext())
            {
                throw new InvalidOperationException("Sequence contains no elements");
            }

            var max = sourceIterator.Current;
            var min = sourceIterator.Current;

            while (sourceIterator.MoveNext())
            {
                var candidate = sourceIterator.Current;

                if (comparer.LeftStrictlyLesserThanRight(candidate, min))
                {
                    min = candidate;
                }

                if (comparer.LeftStrictlyGreaterThanRight(candidate, max))
                {
                    max = candidate;
                }
            }

            return (min, max);
        }
    }

Utilities are defined as below:

internal static class ComparerExtensions
{
    public static bool LeftStrictlyGreaterThanRight<T>(this IComparer<T> comparer, T left, T right)
    {
        return comparer.Compare(left, right) > 0;
    }
    public static bool LeftStrictlyLesserThanRight<T>(this IComparer<T> comparer, T left, T right)
    {
        return comparer.Compare(left, right) < 0;
    }
}

What do you think is the best, knowing that creating a reference type would bring some additional allocations and GC work (I mean that was the whole point to have struct-like tuples in C# 7). What bothers me with the tuple is that fields are not readonly and we cannot prevent users of the method to mess up and break flow.

I don’t know maybe it’s just me and my over-defensive coding style and bad experience using Python where coders are considered as “adults”?

Solution

What bothers me with the tuple is that fields are not readonly and we cannot prevent users of the method to mess up and break flow.

We cannot prevent everything and sometimes it’s just easier and quicker to use the simplest solution like tuples in this case. You can always switch to something more complex if necessary but it’s good to start by following the YAGNI principle.


I was coding some tooling stuff

It depnds on what kind of tooling stuff your are coding. If this is some tiny helper then I’d go with tuples. If I was creatng a general purpose library then and big type would be better. You could validate the parameters and make sure that min <= max etc.

As an example you can take a look at my generic Range.cs that is supported by expressions to be able to work with any comparable type BinaryOperation.cs.


Regarding your ComparerExtensions I start with what I already said in my comment: LeftStrictlyLesserThanRight – I think virtually all programming languages call this just less-than and its counterpart less-than-or-equal.

I also find the extensions are not intuitive. They should be extensions on the T and not on the comparer like that:

x.LessThen(y)

and/or

x.LessThen(y, comparer)

It’s more natural (and shorter) to write than

comparer.LeftLessThenRight(x, y)

This means you can create two pairs of these extensions for each operation.

public static bool LessThen<T>(this T x, T y) where T : IComparable<T>
public static bool LessThen<T>(this T x, T y, IComparer<T> comparer)

one that uses the interface IComparable implemented by T and the other one that uses a custom comparer.

Can use else if on the second

I do not like the name LeftStrictlyLesserThanRight but you could just use that and flop the candidate, max for LeftStrictlyLesserThanRight.

Leave a Reply

Your email address will not be published. Required fields are marked *