HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Composing and executing a customer search

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
executingcomposingsearchandcustomer

Problem

I was wondering if someone could help me refactor this code. Although I know this looks bad, it works. Now I would like to refactor it so I would respect the open/closed principle in SOLID. Let's say the client wants a new search field to search data on, then I need to come to this method and add it in and this breaks the O/C principle. How would you guys change this?

public List Search(Search searchData){
using(context)
{
    var query = context.Customers;

    if(!string.NullOrEmpty(searchData.FirstName)
        query.Where(c => c.FirstName == searchData.FirstName);

    if(!string.NullOrEmpty(searchData.LastName)
        query.Where(c => c.LastName == searchData.LastName);

    if(!string.NullOrEmpty(searchData.StreetName)
        query.Include(c => c.Address);
        query.Where(c => c.Address.StreetName == searchData.StreetName);

    if(!string.NullOrEmpty(searchData.PostCode)
        query.Include(c => c.City);
        query.Where(c => c.City.PostCode == searchData.PostCode);

    var customers = query.ToList();
    return customers;
}    }

Solution

Not sure if you know how dangerous omitting braces for if statements can be. Do you know what happens here :

if(!string.NullOrEmpty(searchData.StreetName)
    query.Include(c => c.Address);
    query.Where(c => c.Address.StreetName == searchData.StreetName);


no matter wether searchData.StreetName is null or not the query.Where(c => c.Address.StreetName == searchData.StreetName); will be executed nevertheless. I assume from the way you have formatted the code that you assume this will be only executed if the condition is true.

I would like to encourage you to always use braces {} although they might be optional. This helps you to structure your code which helps to make it more readable and by using them you make your code less error prone.

Code Snippets

if(!string.NullOrEmpty(searchData.StreetName)
    query.Include(c => c.Address);
    query.Where(c => c.Address.StreetName == searchData.StreetName);

Context

StackExchange Code Review Q#127574, answer score: 10

Revisions (0)

No revisions yet.