patterncsharpMinor
Prettify this code - combine items in a list
Viewed 0 times
thiscombinecodeitemsprettifylist
Problem
Can anyone help me prettify this code? The idea is to combine adjacent time spans so
[0:00-1:00, 1:00-2:00, 3:00-4:00] becomes [0:00-2:00, 3:00-4:00]
[0:00-1:00, 1:00-2:00, 3:00-4:00] becomes [0:00-2:00, 3:00-4:00]
private static IEnumerable CombineAdjacentSpans(IEnumerable spans)
{
var combined = new List();
var list = spans.OrderBy(s => s.Start).ToList();
var candidate = list.FirstOrDefault();
foreach (var span in list.Skip(1))
{
if (span.Start == candidate.End)
{
candidate.End = span.End;
continue;
}
combined.Add(candidate);
candidate = span;
}
if (candidate != null)
combined.Add(candidate);
return combined;
}Solution
You can shorten this and get lazy evalution using yield:
This way, something like mySpans.CombineAdjacentSpans().Take(3) will only do the work of combining the first 3 spans (although OrderBy will still have to look at all elements in the input sequence).
In response to Chris's comment, if you really want the last if statement inside the loop here's one way I can think of to do it:
private static IEnumerable CombineAdjacentSpans(this IEnumerable spans)
{
Span current = null;
foreach (var span in spans.OrderBy(s => s.Start)) {
if (current == null) { current == span; }
// I made this handle overlaps, but if you don't care about those
// you can change the comparison to just ==
else if (span.Start current.End) {
// current.End = span.End
// it's probably better to make a new span rather than modifying the
// current one here since that way this method doesn't modify elements
// in the original list. If span is a struct, then modification would be
// fine but we'd have to rely on a bool rather than checking against null
current = new Span { Start = current.Start, End = span.End };
} else {
yield return current;
current = span;
}
}
if (current != null) { yield return current; }
}This way, something like mySpans.CombineAdjacentSpans().Take(3) will only do the work of combining the first 3 spans (although OrderBy will still have to look at all elements in the input sequence).
In response to Chris's comment, if you really want the last if statement inside the loop here's one way I can think of to do it:
private static IEnumerable CombineAdjacentSpans(this IEnumerable spans)
{
Span current = null;
// could also use Enumerable.Repeat() instead of new[] { ... }
foreach (var span in spans.OrderBy(s => s.Start).Concat(new[] { default(Span) })) {
if (current == null) { current == span; }
// I made this handle overlaps, but if you don't care about those
// you can change the comparison to just ==
else if (span.Start current.End) {
// current.End = span.End
// it's probably better to make a new span rather than modifying the
// current one here since that way this method doesn't modify elements
// in the original list. If span is a struct, then modification would be
// fine but we'd have to rely on a bool rather than checking against null
current = new Span { Start = current.Start, End = span.End };
} else {
yield return current;
current = span;
}
}
}Code Snippets
private static IEnumerable<Span> CombineAdjacentSpans(this IEnumerable<Span> spans)
{
Span current = null;
foreach (var span in spans.OrderBy(s => s.Start)) {
if (current == null) { current == span; }
// I made this handle overlaps, but if you don't care about those
// you can change the comparison to just ==
else if (span.Start <= current.End && span.End > current.End) {
// current.End = span.End
// it's probably better to make a new span rather than modifying the
// current one here since that way this method doesn't modify elements
// in the original list. If span is a struct, then modification would be
// fine but we'd have to rely on a bool rather than checking against null
current = new Span { Start = current.Start, End = span.End };
} else {
yield return current;
current = span;
}
}
if (current != null) { yield return current; }
}private static IEnumerable<Span> CombineAdjacentSpans(this IEnumerable<Span> spans)
{
Span current = null;
// could also use Enumerable.Repeat() instead of new[] { ... }
foreach (var span in spans.OrderBy(s => s.Start).Concat(new[] { default(Span) })) {
if (current == null) { current == span; }
// I made this handle overlaps, but if you don't care about those
// you can change the comparison to just ==
else if (span.Start <= current.End && span.End > current.End) {
// current.End = span.End
// it's probably better to make a new span rather than modifying the
// current one here since that way this method doesn't modify elements
// in the original list. If span is a struct, then modification would be
// fine but we'd have to rely on a bool rather than checking against null
current = new Span { Start = current.Start, End = span.End };
} else {
yield return current;
current = span;
}
}
}Context
StackExchange Code Review Q#15264, answer score: 4
Revisions (0)
No revisions yet.