Sorting a list based on date and time

Question

I have a class that is used to basically store samples from data being read from a system. Within that class is a property that I use to store the date and time of that sample:

public class ReadingValue
{
 public DateTime DateTimeOfReading { get; set; }
 public float Reading { get; set; }
}

I need to sort a list of ReadingValue based on the DateTimeOfReading property.

This is what I've come up with:

public IEnumerable<ReadingValue> Hourly(IList<ReadingValue> readings)
{
 var sortedReadings = readings.OrderBy(x => x.DateTimeOfReading.TimeOfDay)
 .OrderBy(x => x.DateTimeOfReading.Date)
 .OrderBy(x => x.DateTimeOfReading.Year);
 return sortedReadings;
}

The code seems to work, when I pass in a list of ReadingValue's it returns the list in sorted order. Is there anything I've overlooked or should do differently? I know time and date programming can be a tricky topic with things like timezones, daylight savings etc. but sorting dates and times shouldn't involve any of that, I don't think.

Answer 1

You should first do OrderBy and then you should do ThenBy instead of doing another OrderBy. What you are currently doing is that you are re-ordering everything after your first OrderBy.

public IEnumerable<ReadingValue> Hourly(IList<ReadingValue> readings)
{
 var sortedReadings = readings.OrderBy(x => x.DateTimeOfReading.TimeOfDay)
 .ThenBy(x => x.DateTimeOfReading.Date)
 .ThenBy(x => x.DateTimeOfReading.Year);
 return sortedReadings;
}

• Simon Forsberg: This is a good suggestion really, but I think that in this case of the code it will produce the same results, thanks to the current code first doing order by time of day, then of date, then of year.
• Simon Forsberg: This question seems to be related to this: stackoverflow.com/q/3047455/1310566 . Using ThenBy is the correct way to do that.
• Der Kommissar: This isn't quite the same as the OP's code, and will change the order of the list significantly.
• jmoreno: @SimonForsberg: the current version of LINQ to objects uses a stable sort, but other implementations may not. So while you are right that in this instance it doesn't make a difference, that is only coincidentally true. It is not part of the definition for OrderBy, and MS is free to change it at any time, and other C# implementations (Mono for instance) are free to use an unstable sort.
• Simon Forsberg: @jmoreno Agreed. The version with ThenBy is definitely better.

Answer 2

You could simplify it like this -

public IEnumerable<ReadingValue> Hourly(IEnumerable<ReadingValue> readings)
{
 return readings.OrderBy(x => x.DateTimeOfReading);
}

And why is that method called Hourly? Perhaps OrderReadings would be more appropriate.

Answer 3

In addition to @PankajGupta answer I would like to add, that you better should use an IEnumerable<ReadingValue> instead of an IList<ReadingValue> as the methods argument type.

In this way you aren't restricted to pass an IList<> but you can for instance pass any array ReadingValue[], ICollection<ReadingValue> or any object which implements an IEnumerable<ReadingValue>to this method as well.

---

Because methodnames should be made out of verbs or verb phrases you should consider to change the methodname from Hourly to SortHourly().

See: NET naming guidelines