3
\$\begingroup\$

I'm trying to write an extension (method chaining) for playing around a bit.

What I made work is an extension to add an item to the list (what is pretty easy):

IEnumerable<int> myList = originalList.Add(12);

What I now want to do is:

IEnumerable<int> myList = originalList.Add(12).If((list, @new) => !list.Contains(@new));

What it should do: If the originalList doesn't already contain @new (12), then it may not add it to the list. If the If-sequence is not added, the element gets added.

So this is my code:


public static class EnumerableExtension
{
    public static IAddEnumerable<T> Add<T>(this IEnumerable<T> @this, T element)
        => new AddEnumerable<T>(@this, element);
    
}

public interface IAddEnumerable<T> : IEnumerable<T>
{
    IEnumerable<T> If(Func<IEnumerable<T>, T, bool> expression);
}
            
internal class AddEnumerable<T> : IAddEnumerable<T>
{
    private IEnumerable<T> _sequence;
    private readonly T _elementToAdd;

    private bool _added;

    public AddEnumerable(IEnumerable<T> sequence, T elementToAdd)
    {
        _sequence = sequence;
        _elementToAdd = elementToAdd;
    }

    public IEnumerable<T> If(Func<IEnumerable<T>, T, bool> expression)
    {
        _added = true;

        if (expression(_sequence,_elementToAdd))
            _sequence = _sequence.AddItem(_elementToAdd);

        return this;
    }

    public IEnumerator<T> GetEnumerator()
    {
        if (_added) 
            return _sequence.GetEnumerator();

        _sequence = _sequence.AddItem(_elementToAdd);
        _added = true;

        return _sequence.GetEnumerator();
    }
            
    IEnumerator IEnumerable.GetEnumerator()
    {
        if (_added) 
            return _sequence.GetEnumerator();

        _sequence = _sequence.AddItem(_elementToAdd);
        _added = true;

        return _sequence.GetEnumerator();
    }
}

It works, but I wanted to ask, if there are any problems that could come up with, or if there is something I could do better.

\$\endgroup\$
3
  • \$\begingroup\$ The AddEnumerable constructors feels really strange. What's the sole purpose of the _elementToAdd private field? \$\endgroup\$ Commented Jul 30, 2020 at 9:11
  • \$\begingroup\$ @PeterCsala this is the element/item that gets added whether the condition in If is true, or there is no If. \$\endgroup\$ Commented Jul 30, 2020 at 9:23
  • \$\begingroup\$ It still feels strange. You can use it with or without the If builder method. That's why the complexity goes into GetEnumerator methods. You have code duplication because of this. \$\endgroup\$ Commented Jul 30, 2020 at 9:34

1 Answer 1

4
\$\begingroup\$

I like the idea - but have the following comments:

  var originalList = Enumerable.Range(1, 12).ToList();
  IEnumerable<int> myList = originalList.Add(12).If((list, item) => !list.Contains(item));

If the originalList is defined explicit as something that implements ICollection, then you get a compiler error saying: "Operator '.' cannot be applied to operand of type 'void'", because Add() is already a method returning void.


    public IEnumerator<T> GetEnumerator()
    {
        if (_added) 
            return _sequence.GetEnumerator();

        _sequence = _sequence.AddItem(_elementToAdd);
        _added = true;

        return _sequence.GetEnumerator();
    }
            
    IEnumerator IEnumerable.GetEnumerator()
    {
        if (_added) 
            return _sequence.GetEnumerator();

        _sequence = _sequence.AddItem(_elementToAdd);
        _added = true;

        return _sequence.GetEnumerator();
    }

Don't repeat code - the second method can call the first:

IEnumerator IEnumerable.GetEnumerator()
{
  return GetEnumerator();
}

public interface IAddEnumerable<T> : IEnumerable<T>
{
    IEnumerable<T> If(Func<IEnumerable<T>, T, bool> expression);
}

By renaming this interface you could extend its usability to for instance a sequence like:

data.Remove(12).If(<predicate>);

Maybe IPredicate or something like that.


Although I know that @new, @this etc. are valid variable names, I personally always avoid them, because they distract the reading. I have never been in a situation where it was unavoidable to use them.


I understand, that you want a fluid approach, and therefore define the If method separately, but I think, I would concatenate the behavior to a single function like:

public static IEnumerable<T> AddIf<T>(this IEnumerable<T> source, T element, Func<IEnumerable<T>, T, bool> predicate)
{
  return predicate(source, element) ? source.AddItem(element) : source;
}

An idea came to my mind: Because you return this from If(), I think, you can avoid the repetitive code in there by saving the predicate and then handle everything in GetEnumerator():

  internal class AddEnumerable<T> : IAddEnumerable<T>
  {
    private IEnumerable<T> _sequence;
    private readonly T _elementToAdd;
    private Func<IEnumerable<T>, T, bool> _expression;

    private bool _added;

    public AddEnumerable(IEnumerable<T> sequence, T elementToAdd)
    {
      _sequence = sequence;
      _elementToAdd = elementToAdd;
    }

    public IEnumerable<T> If(Func<IEnumerable<T>, T, bool> expression)
    {
      _expression = expression;
      return this;
    }

    public IEnumerator<T> GetEnumerator()
    {
      if (_added)
        return _sequence.GetEnumerator();

      if (_expression == null || _expression(_sequence, _elementToAdd))
      {
        _sequence = _sequence.AddItem(_elementToAdd);
      }
      _added = true;

      return _sequence.GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
      return GetEnumerator();
    }
  }
\$\endgroup\$
1
  • \$\begingroup\$ Great. Yep, I named the method AddE (due to Add already exists) in my file and renamed it for posting here - didn't think about anymore. I only (and always) use @this for extension-methods, otherwise i'm trying to avoid those. Thanks for review and tipps! \$\endgroup\$ Commented Jul 30, 2020 at 11:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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