Writing the condition x <= y AND x > 0

Question

My current condition is written as follows:

.Where(m => Math.Round((m.DateAndTime - DateTime.Now).TotalMinutes, 0) <= 30 && Math.Round((m.DateAndTime - DateTime.Now).TotalMinutes, 0) > 0)

which basically says where the rounded value is less than or equal to 30 and the rounded value is greater than 0.

Is there a neater way to write this than having

Math.Round((m.DateAndTime - DateTime.Now).TotalMinutes, 0)

twice in the same statement?

The full expression is:

List<Meeting> finalList = initialList.Where(m => Math.Round((m.DateAndTime - DateTime.Now).TotalMinutes, 0) <= 30 && Math.Round((m.DateAndTime - DateTime.Now).TotalMinutes, 0) > 0).ToList();

Answer 1

If you need to store a result of a method you should just create a method for it.

private bool IsInRange(DateTime dateTime, decimal min, decimal max)
{
    decimal totalMinutes = Math.Round((dateTime - DateTime.Now).TotalMinutes, 0);
    return totalMinutes > min && totalMinutes <= max;
}  

now your expression looks like so

.Where(m => IsInRange(m.DateAndTime, 0, 30));  

but the values 0 and 30 are still magic numbers which should be extracted to meaningful constants.

The choosen method name is not the best like @Falco has correctly pointed out in his comment

UhhOhhh Horribly wrong named Function.... AreMinutesInRangeFromNow or something like this would be better - with your Method-Name I would expect no Magic happening with DateTime.Now - or pass the already substracted value, but don't hide this!

so you should change it to a better name. I don't like the name @Falco suggested either but couldn't come up with a different one. So it is up to you to choose one.

Answer 2

One way is to use query-style LINQ and the let keyword:

var finalList = (from m in initialList
                 let value = Math.Round((m.DateAndTime - DateTime.Now).TotalMinutes
                 where value <= 30 && value > 0
                 select m).ToList();

Answer 3

Write an IsBetween extension method:

public static bool IsBetween<T>(this T item, T start, T end)
{
    return Comparer<T>.Default.Compare(item, start) >= 0
        && Comparer<T>.Default.Compare(item, end) <= 0;
}

And use it like this:

.Where(m => Math.Round((m.DateAndTime - DateTime.Now).TotalMinutes, 0).IsBetween(0, 30))

(And also take into account the remarks WRT magic numbers and UtcNow etc.)

Answer 4

Use a block-lambda and extract a variable

You can use a block-lambda instead of a statement lambda, and extract a variable:

var now = DateTime.Now;
List<Meeting> finalList = initialList.Where(m => {
    var minutesToNow = Math.Round((m.DateAndTime - now).TotalMinutes, 0);
    return minutesToNow <= 30 && minutesToNow > 0
}).ToList();

Alternatively, use a method like Heslacher suggested.

Use UtcNow

You should avoid using local times, and use Utc times. Prefer DateTime.UtcNow to DateTime.Now.

Answer 5

You can introduce an additional Select where you calculate the offset from Now:

var finalList =
    initialList.Select(m =>
                        new
                            {
                                Meeting = m,
                                MinutesOffsetFromNow = Math.Round((m.DateAndTime - DateTime.Now).TotalMinutes, 0)
                            })
                .Where(x => x.MinutesOffsetFromNow <= 30 && x.MinutesOffsetFromNow > 0)
                .Select(x => x.Meeting)
                .ToList();

And then transform it back to only a Meeting just before the ToList.

---

Doing calculations correctly with time is hard using just DateTime, especially if you have to consider time zone changing as with daylight saving time or users in different time zones.

You should consider if Noda Time can help you express you intents more clearly.

Answer 6

You can simplify your condition, and not use Math.Round (which is really not needed here). It will be more concise and readable:

dt > DateTime.Now && dt <= DateTime.Now.AddMinutes(30)