Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I want to generate a sequence using Linq that goes from 10 to 100 with a step size of 10. The sequence also must contain a custom value (in the correct order).

This is what I have now, and I'm wondering if there is a smarter way of doing it using Linq expressions.

var pageSize = 25; //number that must be in sequnce if not already, can be anything.
var items = Enumerable.Range(10, 100)     //Generate range           
           .Where(x => x % 10 == 0)       //Take every 10th item
           .Concat(new[] { pageSize })    //Add the custom number
           .Distinct()                    //If the custom number was already in there, remove it
           .OrderBy(x => x);              //Sort the sequence

Result:

10 
20 
25 
30 
40 
50 
60 
70 
80 
90 
100 
share|improve this question
    
Why should the result include 25? –  200_success Apr 16 at 7:48
    
@200_success Could be 25 or any other integer. This code is used to generate the datasource to a drop down to select the page size of a GridView. The custom number (25 in the case) is the page size currently set on the gridview and I need to include that in the sequence as that value is set as the default selected value. –  Magnus Apr 16 at 7:52
    
+1 Looks pretty nifty to me –  dreza Apr 16 at 7:54

4 Answers 4

up vote 3 down vote accepted

Know your library

.Distinct() should immediately make you think of a Set.
.OrderBy(...) should make you think of a sorted collection.

I give you: the SortedSet<T>, part of .NET since version 4.0.

When LINQ is not enough, extend it

LINQ uses extension methods, so if it doesn't have what you need, add your own:

private static IEnumerable<T> ToSelectionWith<T>(this IEnumerable<T> sequence, params T[] items)
{
    return new SortedSet<T>(sequence.Concat(items));
}

Usage

This allows you to call it as though it were a part of LINQ itself:

var pageSize = 25;
var items = Enumerable.Range(1, 10).Select(x => x*10).ToSelectionWith(pageSize);

Benefits

  1. It's concise — a one-liner. You could even skip the extension method and use the SortedSet directly.
  2. It's readable — by wrapping it in the extension method, the intent of using the SortedSet is clarified.
  3. It performs well — in my crude Stopwatch testing, it beat your and Sandeep's versions for both small and large input sequences. (Specifically, the SortedSet based approach runs twice as fast as Sandeep's version for your original input, and at least three times as fast for very large input sizes).
share|improve this answer
    
I disagree with your second point. It is not readable at all. If i were to see the line of code from your usage example, i would assume that it adds an element to enumeration. Certainly not that it both sorts and distincts it as well. You extension method does not indicate the usage of SortedSet in its implementation in any way. –  Nikita Brizhak Apr 17 at 6:48
    
@Nikita okay, I tend to agree the name isn't perfect. I changed it to ToSelectionWith(pageSize), which I think is slightly more descriptive and less misleading. The point of an abstraction, though, is to keep this implemention detail hidden away. –  codesparkle Apr 17 at 7:10
    
It is indeed. That is why the name should be descriptive enough, to call the method readable. I do not suggest to call it ToSortedSetWith() or something (though i would consider something along these lines), i merely point out that its name (at least the old one) does not reflect what this method actually does. –  Nikita Brizhak Apr 17 at 8:07
1  
+1 for favoring readability of business logic over total LOC. I also liked using a method to replace an idiom, namely Concat(new{...}). However Selection has no inherent meaning. Also result being a sorted set is not really an implementation detail, as it being a distinct and ordered collection is a requirement. How about refactoring your ToSelectionWith to SRP methods like someEnumerable.Append(someValue, someOtherValue).ToSortedSet(). With ToSortedSet being a least surprise counterpart to ToArray and ToList. –  abuzittin gillifirca Apr 17 at 11:40
    
Using a sorted set is a good idea. But ToSelectionWith is badly named and nothing more than UnionAndSort, so it's an SRP violation as well. –  CodesInChaos Apr 17 at 11:58

Size of initial list in the following code is less:

var x = Enumerable.Range(1, 10)
                  .Select(x_ => x_ * 10)
                  .Concat(new[] {25})
                  .Distinct()
                  .OrderBy(x_ => x_)
                  .ToList();

Otherwise your original code is perfect.

share|improve this answer
    
Where is this x_ notation coming from? I have never seen it in C#. –  Nikita Brizhak Apr 16 at 8:15
    
Also, strictly speaking Range does not create list. Your version does require less iterations nonetheless and is easier to read. –  Nikita Brizhak Apr 16 at 8:21
1  
This is the coding convention that I follow. x_ is used for input parameters and _x is that is used for member variables. –  Sandeep Apr 16 at 8:41
3  
@Chris, well, obviously. I was asking what naming convention is that (as it contradicts msdn guidelines for parametrs). –  Nikita Brizhak Apr 17 at 6:40
1  
You can collapse Concat(...).Distinct into Union –  CodesInChaos Apr 17 at 11:59

Just include the custom value in the condition, then you don't need to concatentat it, remove duplicates or sort. To get the range from 10 to 100 you should use 91 as count in the Range method, Range(10, 100) creates numbers from 10 to 109.

var pageSize = 25;

var items =
  Enumerable.Range(10, 91)           
  .Where(x => x % 10 == 0 || x == pageSize);

Note: This naturally only works if the custom value is inside the range.

share|improve this answer
    
This is a neat way. Unfortunately the pageSize could be outside of the range. –  Magnus Apr 16 at 9:23

Although there isn't much left to add to the actual use case OP mentioned, considering the general problem of inserting a value in the correct order in an ordered sequence, a more time and space efficient solution exists.

public static IEnumerable<T> InsertPreservingOrder<T> (this IEnumerable<T> sortedSequence, T value)
    where T : IComparable<T>
{
    return sortedSequence.TakeWhile(x => x.CompareTo(value) < 0)
             .Append(value)
             .Concat(sortedSequence.SkipWhile(x => x.CompareTo(value) <= 0));
}

where Append is syntactic sugar for .Concat(new{...}) as per @codesparkle's answer:

public static IEnumerable<T> Append<T>(this IEnumerable<T> sequence, params T[] items)
{
    return sequence.Concat(items);
}

This operation is of constant (O(1)) space, linear (O(n)) time and lazy. Especially useful in situations like this :

var customValue = 15;
var defaultValues = Enumerable.Range(1, 10000000).Select(x_ => x_ * 10);
var items = defaultValues.InsertPreservingOrder(customValue);
// then later
items.Take(10).ToList().ForEach(Console.WriteLine);

I also think this method is a clear improvement in capturing programmer's intent w.r.t. the original version, regardless of asymptotic complexity. It reifies an idea; therefore can be reused, whenever the current value of a variable would be added in a collection of default options. Its name makes clear both the precondition that first parameter is assumed to be already sorted, and the postcondition that the return value is also sorted. However it leaves the distinctness condition opaque.

Though I'd still prefer a simple defaultValues.Append(customValue).ToSortedSet()

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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