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.
1 Answer 1
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();
}
}
-
\$\begingroup\$ Great. Yep, I named the method
AddE
(due toAdd
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\$Matthias Burger– Matthias Burger2020年07月30日 11:08:55 +00:00Commented Jul 30, 2020 at 11:08
AddEnumerable
constructors feels really strange. What's the sole purpose of the_elementToAdd
private field? \$\endgroup\$If
is true, or there is noIf
. \$\endgroup\$If
builder method. That's why the complexity goes intoGetEnumerator
methods. You have code duplication because of this. \$\endgroup\$