7

I have the following code

    return
    this.Storage.Customer.OfType<Preferred>()
    .Include(b  => b.Order)
    .Where(cust => cust.Id == customerId && cust.CustomerType== (int)cusType)
    .SingleOrDefault();

It can be rewritten as follows eliminating the where.

    return
    this.Storage.Customer.OfType<Preferred>()
    .Include(b  => b.Order)
    .SingleOrDefault(cust => cust.Id == customerId && cust.CustomerType == (int)cusType);

Which one is better practice and why?

MicroMan
  • 1,988
  • 4
  • 32
  • 58

3 Answers3

2

Thanks to the complexity of debugging the return value of functions, and the impossibility of using lambda expressions in the debugger, this is the best way:

var temp = this.Storage.Customer.OfType<Preferred>()
    .Include(b  => b.Order)
    .Where(cust => cust.Id == customerId && cust.CustomerType == (int)cusType);

return temp.SingleOrDefault();

In this way, if there is an exception on the SingleOrDefault() (something quite common if you are doing complex expressions), you can put a breakpoint on the return and do in the watch panel: temp.ToList();

xanatos
  • 109,618
  • 12
  • 197
  • 280
  • 4
    With all due respect, this is an opinion. – Maarten Aug 27 '13 at 09:01
  • 1
    @Maarten I'll call it first-hand experience :-) My code is better than the two versions suggested by the OP because mine has a verifiable advantage. The resulting code is equivalent/nearly equivalent to the one of OP (at the IL level), it's as equally readable and is easier to debug. – xanatos Aug 27 '13 at 09:05
1

First of all you need to understand the difference

this.Storage.Customer.OfType<Preferred>()
.Include(b  => b.Order)
.Where(cust => cust.Id == customerId && cust.CustomerType== (int)cusType)

this will just create a query but will not execute untill you call ToList method.

SingleOrDefault method will actually execute the query. So if you want to check or do something with the query before it is executed you should use where and then call the SingleOrDefault.

so overall, using where is good practice as per my personal opinion

Ronak Patel
  • 2,570
  • 17
  • 20
0

Old post and this is largely opinion-based, but here's another consideration not mentioned above:

A SingleOrDefault clause can't be followed by other terms like Select because, as Patel notes above, it actually executes the query. For example say down the road you want to modify the query to return just the customer's name:

this.Storage.Customer.OfType<Preferred>()
.Include(b  => b.Order)
.SingleOrDefault(cust => cust.Id == customerId && cust.CustomerType == (int)cusType)
.Select(cust=>cust.Name); //compile error

won't work, and you can't move the Select clause to before the SingleOrDefault because then the Id and CustomerType fields won't be visible to the lambda expression in SingleOrDefault. Instead you'd need to add back the Where clause first:

this.Storage.Customer.OfType<Preferred>()
.Include(b  => b.Order)
.Where(cust => cust.Id == customerId && cust.CustomerType == (int)cusType)
.Select(cust=>cust.Name)
.SingleOrDefault();

Thus because the Where clause often is necessary and always at least as good performance, it is probably good practice to always use it for consistency, readability, and maintainability.

Matthew
  • 4,149
  • 2
  • 26
  • 53