How to solve a SonarQube S3267 warning, "Loops should be simplified with "LINQ" expressions"

Question

Consider the following code (which is a simplified version of my actual code):

static void Main(string[] args)
{
    string[] data = { "one", "two", "three", "stop", "four", "five" };

    foreach (string item in data) // S3267 
    {
        if (!process(item))
            break;
    }
}

static bool process(string text)
{
    if (text == "stop")
        return false;

    Console.WriteLine(text);
    return true;
}

At the indicated line, this provokes a SonarQube S3267 warning, "Loops should be simplified with "LINQ" expressions". For now, I have just suppressed this warning.

My question is this: Is there a good way to rewrite this which is as readable?

Resharper suggests this:

foreach (var item in data.Where(item => !process(item)))
{
    break;
}

but that just provokes SonarQube warning "Loops with at most one iteration should be refactored" (or similar), and I think that's less readable than the original version.

I also thought of:

foreach (var item in data.TakeWhile(process))
{
}

but that obscures things in my opinion.

There's also

_ = data.All(process);

but now we're just getting silly.

I'm also considering adding my own extension methods:

public static class MyEnumerableExt
{
    #pragma warning disable S3267 // Loops should be simplified with "LINQ" expressions

    public static void ProcessUntil<T>(this IEnumerable<T> sequence, Func<T,bool> process)
    {
        foreach (var element in sequence)
        {
            if (process(element))
                break;
        }
    }

    public static void ProcessWhile<T>(this IEnumerable<T> sequence, Func<T, bool> process)
    {
        foreach (var element in sequence)
        {
            if (!process(element))
                break;
        }
    }

    #pragma warning restore S3267 // Loops should be simplified with "LINQ" expressions
}

Then the call site would just be:

data.ProcessWhile(process);

but I don't really like adding too many extension methods, especially if they are only used in very limited situations.

Is there any nicer way to resolve this? (I realise that many answers to this question would be a matter of opinion, but I'm looking for a concrete unopinionated suggestion.)

(I think just suppressing the warning is the right approach, but I'm open to suggestions!)

Answer 1

I am not sure I would call this an improvement but it does avoid the use of break in a foreach. It makes me think it would be nice if there was an expression version of foreach like the new expression switch that would let you use enumeration in other looping contexts, like while.

var dataEnum = ((IEnumerable<string>)data).GetEnumerator();
while (dataEnum.MoveNext() && process(dataEnum.Current))
    ;

If your real data is not an array, you wouldn't need the cast, which is required since the non-generic version of GetEnumerator was added to arrays before generic GetEnumerator was added to the language, and they chose not to have the type-safe version be preferred on arrays.

If C# did have an expression foreach, I would have it work like this:

while (data foreach var item && process(item))
    ;

You could have an out after foreach but I think that can be implied. Since I can't figure out a way to have a stateful method in C#, this extension method is as close an implementation as I can come up with:

public static class IEnumerableExt {
    public delegate bool ForEachDel<T>(out T item);

    public static ForEachDel<T> NewForeach<T>(this IEnumerable<T> src) {
        var e = src.GetEnumerator();
        return (out T item) => {
            var res = e.MoveNext();
            if (res)
                item = e.Current;
            else
                item = default;

            return res;
        };
    }
}

Which you would use like:

var dataForeach = data.NewForeach();
while (dataForeach(out var item) && process(item))
    ;