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();

I really wanted to comment on all the answers, but I have always found it easier to read 0 < value && value <= 30 in those languages that don't allow you to write 0 < value <= 30 It makes it much clearer that you're testing for something to be in a specified range. — Tom Tanner
– Tom Tanner, Commented Jul 22, 2015 at 12:46
@TomTanner, a valid point and I'd normally do the same. I kept my order the same as the OP's though to avoid causing possible confusion. — David Arno
– David Arno, Commented Jul 22, 2015 at 12:55

Community · Accepted Answer · 2017-04-13 12:40:57Z

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.

Oh, weird, I always get errors like Method 'Boolean functionName(System.String, System.String)' has no supported translation to SQL. I figured you just couldn't call functions in linq queries. — Charles Clayton
– Charles Clayton, Commented Jul 22, 2015 at 21:50
I would expect min to come before max in the parameter list. — BlueRaja - Danny Pflughoeft
– BlueRaja - Danny Pflughoeft, Commented Jul 22, 2015 at 22:09
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! — Falco
– Falco, Commented Jul 23, 2015 at 8:32
@crclayton You can call functions in linq queries as long as you're not querying the database itself in your query, otherwise it thinks you're trying to translate things to SQL, so what I do is pull the list from the db, then run linq queries on that list object. — barnacle.m
– barnacle.m, Commented Jul 23, 2015 at 10:18

David Arno · Accepted Answer · 2015-07-22 10:51:34Z

15

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();

edited Jul 22, 2015 at 10:51

answered Jul 22, 2015 at 10:30

David Arno

7014 silver badges9 bronze badges

\$\begingroup\$ Why would you rewrite it to query-style, when the asker clearly prefers method-style? \$\endgroup\$

ANeves
– ANeves

2015-07-22 12:46:11 +00:00
Commented Jul 22, 2015 at 12:46
3

\$\begingroup\$ @ANeves, why do you assume the OP prefers the fluent style? I use both styles, depending on the requirement. Further, these answers aren't just for the benefit of the OP, but for others who come searching with similar questions. They may prefer the query syntax; or they may not. Having said all that, if the OP doesn't like using query syntax, then Johnbot has a viable alternative solution that sticks to the fluent style. \$\endgroup\$

David Arno
– David Arno

2015-07-22 12:53:05 +00:00
Commented Jul 22, 2015 at 12:53
\$\begingroup\$ I agree that I should not assume preference. But it is what he is using in the question; I still think that using query-style distracts from the actual solution. \$\endgroup\$

ANeves
– ANeves

2015-07-22 12:56:52 +00:00
Commented Jul 22, 2015 at 12:56
1

\$\begingroup\$ @ANeves there's not really an equivalent to this using fluent syntax and the closest way is a bit clunky. As far as function goes, this is the most elegant. Form-wise, it's up to the individual's aesthetic preference. \$\endgroup\$

moarboilerplate
– moarboilerplate

2015-07-22 17:47:40 +00:00
Commented Jul 22, 2015 at 17:47
1

\$\begingroup\$ @moarboilerplate If you look at the IL from this answer and mine you'll see that they're identical except for the anonymous type and delegate names. \$\endgroup\$

Johnbot
– Johnbot

2015-07-23 09:17:50 +00:00
Commented Jul 23, 2015 at 9:17

| Show 6 more comments

Community · Accepted Answer · 2017-05-23 12:40:52Z

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.)

Community · Accepted Answer · 2017-04-13 12:40:57Z

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.

Good idea on avoiding local times if I ever want my software to scale internationally! — barnacle.m
– barnacle.m, Commented Jul 23, 2015 at 10:16

Heslacher · Accepted Answer · 2015-07-22 11:55:26Z

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.

SergeyS · Accepted Answer · 2015-07-23 08:49:20Z

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)

Oh there's a very important reason I use Math.Round, but that has nothing to do with the question — barnacle.m
– barnacle.m, Commented Jul 23, 2015 at 10:17

Stack Exchange Network

Writing the condition x <= y AND x > 0

6 Answers 6

Use a block-lambda and extract a variable

Use UtcNow

You must log in to answer this question.

Hot Network Questions

Writing the condition x <= y AND x > 0

6 Answers 6

Use a block-lambda and extract a variable

Use UtcNow

You must log in to answer this question.

Related

Hot Network Questions