Is was looking at some code with smell. The method description was clear, but the implementation was not.
The code was, for a given sequence to determine if another sequence is present.
After getting some
characterization tests up and running, it was time to do some refactoring.
From 38 lines of code, I was down to this:
for (;;)
public static bool Contains(List<T> x, IList<T> y)
{
for (int index = -1; (index = x.IndexOf(y[0], index + 1)) >= 0;)
{
if (x.Skip(index).Take(y.Count).SequenceEqual(y))
return true;
}
return false;
}
I thought I was done, but there was room for making this smaller. First out was to try to split up the algorithm into multiple parts
yield
public static bool Contains(List<T> x, IList<T> y)
{
return FindStarts(x, y[0])
.Any(index => x.Skip(index).Take(y.Count).SequenceEqual(y));
}
private static IEnumerable<int> FindStarts(List<T> x, T item)
{
for (int index = -1; (index = x.IndexOf(item, index + 1)) >= 0; )
yield return index;
}
Then combine these two methods:
public static bool Contains(IEnumerable<T> x, IList<T> y)
{
return x.Select((value, index) => new { Value = value, Index = index })
.Where(z => z.Value.Equals(y[0]))
.Select(z => z.Index)
.Any(index => x.Skip(index).Take(y.Count).SequenceEqual(y));
}
Another possibility is check each element in the source sequence and drop the IndexOf(,..) logic:
Range
public static bool Contains(IEnumerable<T> x, IList<T> y)
{
return Enumerable.Range(0, x.Count() - 1)
.Any(i => x.Skip(i).Take(y.Count).SequenceEqual(y));
}
And the final one using:
SkipWhile
public static bool Contains(IEnumerable<T> x, IList<T> y)
{
return x.SkipWhile((value, index) => (value.Equals(y[0])
&& x.Skip(index).Take(y.Count).SequenceEqual(y)) == false).Any();
}