Get values between two dates

Question

I have a search form where users can enter ValidFrom and ValidTo dates,

There are three conditions:

1. ValidFrom and ValidTo NOT NULL

   Action: Get values between two dates

2. ValidFrom is NOTNULL and ValidTo is NULL

   Action: Get values where date is greater than ValidFrom

3. ValidFROM is NULL and ValidTo is NOTNULL

   Action: get values where date is less than ValidTo

This is what I have got:

 if (model.ValidFrom != null && model.ValidTo != null)
 {
 var dateFrom = (DateTime)model.ValidFrom;
 var dateTo = (DateTime)model.ValidTo;
 dalList = dalList.Where(s =>
 DbFunctions.TruncateTime(s.ValidFrom) >= dateFrom.Date
 && DbFunctions.TruncateTime(s.ValidTo) <= dateTo.Date).AsQueryable();
 }
 if (model.ValidFrom != null && model.ValidTo == null)
 {
 var dateFrom = (DateTime)model.ValidFrom;
 dalList = dalList.Where(s => DbFunctions.TruncateTime(s.ValidFrom) >= dateFrom.Date).AsQueryable();
 }
 if (model.ValidFrom == null && model.ValidTo != null)
 {
 var dateTo = (DateTime)model.ValidTo;
 dalList = dalList.Where(s => DbFunctions.TruncateTime(s.ValidTo) <= dateTo.Date).AsQueryable();
 }

Is there a better way to write this type of search?

Answer 1

Although the answer from Heslacher is a really good one, here's also what you might do. Create two boolean variable to check for null on the model.ValidFrom and model.ValidTo:

bool modelValidFromIsNull = model.ValidFrom == null;
bool modelValidToIsNull = model.ValidTo == null;

Now, like Heslacher mentioned, place a guard to rule out if they are both null.

if (modelValidFromIsNull && modelValidToIsNull) { return null; }

Now you can use these bool variables to do some magic. In the Where() clause of the query, use these flags to check or ignore the condition you want to apply. If the flag is true, the condition will be skipped and if it is false, the condition will be evaluated. A small example:

var numbers = new[] { 1, 2, 3, 4, 5, 6 };
var remaining = numbers.Where(x => false || x > 3);

The variable remaining will now contain 4, 5 and 6. If you were to change that false to true, the variable remaining would contain all the numbers from the original source. [That's why you need the guard :) ]

Now you can apply this logic to your code. When the model.ValidFrom is not null, your variable modelValidFromIsNull will be false. This means that in the where clause the left part is false and the second part will be evaluated. The same goes for model.ValidTo.

This is the resulting code:

bool modelValidFromIsNull = model.ValidFrom == null;
bool modelValidToIsNull = model.ValidTo == null;
if (modelValidFromIsNull && modelValidToIsNull) { return null; }
var result = dalList.Where(s => modelValidFromIsNull || DbFunctions.TruncateTime(s.ValidFrom) >= dateFrom.Date)
 .Where(s => modelValidToIsNull || DbFunctions.TruncateTime(s.ValidTo) <= dateTo.Date);

Answer 2

If you first check if both are null as a guard condition, then only one of these 3 conditions can be true so you could change this 3 single if statements to a if..else if..else.

if (model.ValidFrom == null && model.ValidTo == null) { return null;}
if (model.ValidFrom != null && model.ValidTo != null)
{
 var dateFrom = (DateTime)model.ValidFrom;
 var dateTo = (DateTime)model.ValidTo;
 dalList = dalList.Where(s =>
 DbFunctions.TruncateTime(s.ValidFrom) >= dateFrom.Date
 && DbFunctions.TruncateTime(s.ValidTo) <= dateTo.Date).AsQueryable();
}
else if (model.ValidFrom != null && model.ValidTo == null)
{
 var dateFrom = (DateTime)model.ValidFrom;
 dalList = dalList.Where(s => DbFunctions.TruncateTime(s.ValidFrom) >= dateFrom.Date).AsQueryable();
}
else
{
 var dateTo = (DateTime)model.ValidTo;
 dalList = dalList.Where(s => DbFunctions.TruncateTime(s.ValidTo) <= dateTo.Date).AsQueryable();
} 

You also could introduce an enum which defines the type of the query to be performed and a method GetSearchType() which returns this enum based on the passed in objects.

private enum SearchType
{
 None, Between, GreaterThanFrom, LessThanTo
}
private SearchType GetSearchType(object fromDate, object toDate)
{
 if (fromDate == null && toDate == null) { return SearchType.None; }
 if (fromDate == null) { return SearchType.LessThanTo; }
 if (toDate == null) { return SearchType.GreaterThanFrom; }
 return SearchType.Between;
} 

then you can use a switch..case like

switch (GetSearchType(model.ValidFrom, model.ValidTo))
{
 case SearchType.Between:
 //search here
 break;
 case SearchType.GreaterThanFrom:
 //search here
 break;
 case SearchType.LessThanTo:
 //search here
 break;
 case SearchType.None:
 //nothing to do here, you could also skip
 break;
}

which is easier to understand. Instead of doing the search at the cases you should for readability consider to extract the searches to separate methods.

Also copy and paste is ok, you should after deleting a line also delete the line break

 var dateFrom = (DateTime)model.ValidFrom;
 dalList = dalList.Where(s => DbFunctions.TruncateTime(s.ValidFrom) >= dateFrom.Date).AsQueryable();

• RobH: Is there a reason why you cast the Nullable<DateTime> to DateTime rather than accessing the Value property? Interested to know whether it's just a style thing, or whether there's a concrete reason one way or the other.
• Heslacher: @RobH Good point. I just used the same style like the OP.

Answer 3

If you add a guard for both being null you can simplify by coalescing your nulls.

if (model.ValidFrom == null && model.ValidTo == null) 
{ 
 throw new InvalidOperationException();
}
var fromDate = model.ValidFrom ?? DateTime.MinValue;
var toDate = model.ValidTo ?? DateTime.MaxValue;
dalList = dalList.Where(s =>
 DbFunctions.TruncateTime(s.ValidFrom) >= dateFrom.Date
 && DbFunctions.TruncateTime(s.ValidTo) <= dateTo.Date).AsQueryable();

As long as you are using a provider that handles the full date range of .Net's DateTime type ( 01/01/0001 to 31/12/9999 ).

You could also use the GetValueOrDefault(T default) method of Nullable<T>:

var fromDate = model.ValidFrom.GetValueOrDefault(DateTime.MinValue);
var dateTo = model.ValidTo.GetValueOrDefault(DateTime.MaxValue);

Of course, if you remove the guard this code will allow you to specify neither and return all results (after min date but before max date == all).

As always, there are lots of ways to skin a cat. Note that I wrote the above straight into code review so it might not work or compile :)

I would imagine that the solution given by Abbas would be more performant though.