C# implement Iterator Pattern and early return empty enumerable if one iterator is not valid

Question

I want to extract two foreach´s which iterate over mostly the same list but one has an early exit if one element is not valid and the other will only be executed if the other collected the data correctly. Therefor I want to use the iterator-pattern.

Thats a scheme of the use case:

foreach (var material in matrix.Materials)
        {
            var validationResult = materialValidator.Validate(material);
            if (validationResult.IsValid == false)
            {
                return;
            }

            validMaterials.Add(
                new Tuple<IMaterial, string>(
                    materialValidator.CurrentMaterial,
                    material.TextMask));
        }

        foreach (var tuple in validMaterials)
        {
            var material = tuple.Item1;
            var text = tuple.Item2;

            material.SetSelcted(true);

            if (string.IsNullOrWhiteSpace(text) == false)
            {
                module.SetTextMask(textMask);
            }
        }

But what I want is to iterate the enumerable if there are valid materials e.g:

    var enumerable = new MaterialsEnumerable(matrix.Materials)
    foreach (var iterator in enumerable)
    {
        iterator.Material.SetSelcted(true);

        if (string.IsNullOrWhiteSpace(iterator.Text) == false)
        {
            module.SetTextMask(iterator.Text);
        }
    }

And the implementation of MaterialsEnumerable so good as follows, with one missing detail: how do I yield return an empty enumerable at one path?

public IEnumerator<MaterialIterator> GetEnumerator()
    {
        foreach (var material in this.materials)
        {
            var result = this.validator.Validate(material);
            if (result.IsValid == false)
            {
                return Enumerable.Empty<MaterialIterator>(); // <- this doesn´t work
            }

            yield return new MaterialIterator(
                this.validator.CurrentMaterial,
                material.TextMask);
        }
    }

Answer 1

My Solution is to yield break if one element is invalid:

public IEnumerator<MaterialIterator> GetEnumerator()
    {
        if (this.materials.Any(material => this.validator.Validate(material).IsValid == false))
        {
            yield break;
        }

        foreach (var material in this.materials)
        {
            yield return new MaterialIterator(
                this.validator.CurrentMaterial,
                material.TextMask);
        }
    }

Answer 2

You can combine the two loops when you continue if the material is not valid.

 foreach (var material in matrix.Materials)
        {
            var validationResult = materialValidator.Validate(material);
            if (!validationResult.IsValid)
            {
                continue;
            }
    
            material.SetSelected(true);
            if (!string.IsNullOrWhiteSpace(material.TextMask))
            {
                module.SetTextMask(material.TextMask);
            }
    
            yield return material;
        }

Answer 3

Ok first, your 'solution' above is overly complex and will cause bugs down the road. Your original attempt also has extra memory in use and cpu cycles (although C# .Net 7 will reduce this issue, it's bad practice) as you are doing 2 loops in place of 1.

First you need to simplify your code, pull that validation function out of your data object (make it a static extension of the object maybe).

public static bool IsValid(this Material material)
{
    //The checking code to check the material is valid
}

So if you JUST want to process only lists with only valid samples in them (as described in the OP) then you refactor your loop and code something like:

if(matrix.Materials.Any(x=> !x.IsValid())
{
    throw new Exception("Maybe log the thrown issues, or you could do something else with it");
}

foreach(var item in matrix.Materials) 
{
    //Do the stuff you want to do to each element in lists that only contain valid materials
}

If you wanted all the valid materials from the list (skipping any invalid materials) it would be more like the following without the first if statement.

foreach(var item in matrix.Materials.Where(x => x.IsValid())
{
    //do stuff for each valid material
}

Always make sure to put you loop conditions in the loop statement, break is ok in some rare circumstances, but you should never use continue as it means you are allocating memory when you do not need to be in any foreach loop and you are breaking the loop control pattern.

Also when you come back after 6 months to edit this, it will be much easier to see what the loop is doing especially if your continue/break is not the first element in the loop.

Answer 4

If I've understood correctly, you don't want any of the calls to material.SetSelcted(true); made if any of the materials are not valid.

There is no way to do this without traversing the (or a) list twice.

We can make the code a little cleaner while still having it fail early.

Assuming that matrix.Materials is an array, list, or some other ICollection<Material> then this is quite efficient.

var count =
    matrix
        .Materials
        .TakeWhile(x => materialValidator.Validate(x).IsValid)
        .Count();

if (matrix.Materials.Count() == count)
{
    foreach (var material in matrix.Materials)
    {
        material.SetSelcted(true);
        if (string.IsNullOrWhiteSpace(material.TextMask) == false)
        {
            module.SetTextMask(material.TextMask);
        }
    }
}

The TakeWhile allows the iteration to stop early. If the counts match then the entire list was checked and we can proceed on the original collection as it's all been validated.

This approach doesn't create any temporary list and is efficient as possible.