views:

170

answers:

6

I have a LINQ method for the Search page of an in house app. The method looks as below

    public static DataTable SearchForPerson(String FirstName, String MiddleName, String LastName, String SSN, DateTime? BirthDate)
    {
        var persons = (from person in context.tblPersons 
                       where person.LastName == LastName || person.LastName.StartsWith(LastName)
                       join addresse in context.tblAddresses on person.PersonID equals addresse.PersonID 
                       orderby person.LastName
                       select new { person.PersonID, person.LastName, person.FirstName, person.SSN, addresse.AddressLine1 });

        var filteredPersonsList = persons.Where(p => p.LastName == LastName).ToList();
        if (filteredPersonsList.Count == 0)
            filteredPersonsList = persons.Where(p => p.LastName.StartsWith(LastName)).ToList();

        var dataTable = filteredPersonsList.CopyLinqToDataTable();



        return dataTable;
    }

Now, as you can no doubt see, I made a slight oversight when creating this as it only searches by LastName. I was in process of expanding this when it occured to me that I may not be going about this properly.

So, finally to my question; Is it more desirable(read-best practice, more efficient, etc...) to have a single method like this with a mechanism(I am thinking a SWITCH on non-empty param) to tell which parameter to search with or should I simply make multiple versions, a la SearchForPersonByLastName & SearchForPersonBySSN?

Additionally, is there an even more elagent solution to this, I would think common, issue?

+2  A: 

Am I understanding correctly that only one of the parameters will be used to do the search? If so then absolutely these should be separate methods. Any time you describe a method (or class etc.) using the word "and" or "or" you probably have a method that can be broken into multiple methods. So it sounds like this method is currently described as "this method searches for Persons by FirstName or MiddleName or LastName or SSN or BirthDate." So, write methods

SearchByFirstName
SearchByMiddleName
SearchByLastName
SearchBySSN
SearchByBirthDate

Obviously there will be some common logic between these methods that you can factor out into a helper method.

Please clarify if I misunderstood and I'll edit my answer accordingly.

Edit:

Okay, so you say that you might be searching by multiple parameters. I still strongly prefer the idea of separate methods for each parameter (better separation of concerns, easier to maintain, easier to test, etc.). Here is one way to tie them all together:

DataTable Search(
    string firstName,
    string middleName,
    string lastName,
    string ssn,
    DateTime? birthdate
) {
    IQueryable<Person> query = context.tblPersons;
    if(SearchParameterIsValid(firstName)) {
        query = SearchByFirstName(query, firstName);
    }
    if(SearchParameterIsValid(middleName)) {
        query = SearchByMiddleName(query, middleName);
    }
    if(SearchParameterIsValid(lastName)) {
        query = SearchByLastName(query, lastName);
    }
    if(SearchParameterIsValid(ssn)) {
        query = SearchBySSN(query, ssn);
    }
    if(birthDate != null) {
        query = SearchByBirthDate(query, birthDate);
    }

    // fill up and return DataTable from query
}

bool SearchParameterIsValid(string s) {
    return !String.IsNullOrEmpty(s);
}

IQueryable<Person> SearchByFirstName(
    IQueryable<Person> source
    string firstName
) {
    return from p in source
           where p.FirstName == firstName || p.FirstName.StartsWith(firstName)
           select p;
}

// etc.

Or:

DataTable Search(
    string firstName,
    string middleName,
    string lastName,
    string ssn,
    DateTime? birthdate
) {
    Predicate<Person> predicate = p => true;
    if(SearchParameterIsValid(firstName)) {
        predicate = PredicateAnd(predicate, FirstNamePredicate(firstName));
    }
    if(SearchParameterIsValid(middleName)) {
        predicate = PredicateAnd(predicate, MiddleNamePredicate(middleName));
    }
    // etc.
}

Predicate<T> PredicateAnd<T>(Predicate<T> first, Predicate<T> second) {
    return t => first(t) && second(t);
}

Predicate<Person> FirstNamePredicate(string firstName) {
    return p => p.FirstName == firstName || p.FirstName.StartsWith(firstName);
}

// etc.

DataTable SearchByPredicate(
    IQueryable<Person> source,
    Predicate<Person> predicate
) {
    var query = source.Where(predicate)
                      .Join(
                          context.tblAddresses,
                              p => p.PersonID,
                              a => a.PersonID,
                              (p, a) => new {
                                  p.PersonID,
                                  p.LastName,
                                  p.FirstName,
                                  p.SSN,
                                  a.AddressLine1
                              }
                          );

    return query.CopyLinqToDataTable();
}
Jason
No that is mostly correct. My only concern is for the edge cases where they would want to Search by multiple parameters, have even more methods, `SearchByFirstAndLastName()`??
Refracted Paladin
Please see my edit.
Jason
+1  A: 

If I understand your question right, you're trying to add the other parameters to the where clause of your query. Might I suggest:

var persons = (from person in context.tblPersons 
                       where (!string.IsNullOrEmpty(LastName) && (person.LastName == LastName || person.LastName.StartsWith(LastName))) && 
                       (!string.IsNullOrEmpty(SSN) && (person.SSN == SSN)) // && etc as needed
                       join addresse in context.tblAddresses on person.PersonID equals addresse.PersonID 
                       orderby person.LastName
                       select new { person.PersonID, person.LastName, person.FirstName, person.SSN, addresse.AddressLine1 });

This would allow you to pass any combination of parameters to filter on so you're not locked in to filtering on one parameter.

ddc0660
+1  A: 

The intent would be much more clear with multiple methods.

If I look at your code and you use just one method, I'd be able to figure out what was going on, but I'd have to look at it for a while to see what the heck you're doing. Maybe some comments would help clarify things, etc...

But, multiple methods will show me EXACTLY what you're trying to do.

As Jason said, be sure to factor out the common code into a helper method. I'd hate to see the same (more or less) linq query in each method.

Terry Donaghe
+1  A: 

You can add multiple where clauses so callers can specify the name fields they wish to search on, or null to match anything:

var filteredPersonsList = persons
    .Where(p => FirstName != null && p.FirstName == FirstName)
    .Where(p => MiddleName != null && p.MiddleName == MiddleName)
    .Where(p => LastName != null && p.LastName == LastName).ToList();

So the caller could specifiy:

var matches = SearchForPerson("firstName", null, "lastName", "SSN", dob);

To ignore the middle name in the search.

Note you can combine these clauses into one using && although that could get difficult to read.

Lee
+1  A: 

The single method you have is fine.

I would build up the LINQ one where clause at a time. This way when you actually run the LINQ it only processes the needed where clauses. This should be more efficient than other solutions proposed. LINQ is great as you can create your LINQ expression piece by piece using if logic as needed and then run it. You do not need to put all your if logic inside your LINQ expression if you can determine the logic while building the LINQ expression.

I would also simplify to only StartsWith.

One other thing, it seems that filteredPersonsList filtering is redundant as you are already filtering and I believe you can get rid of those lines.

var persons = from person in context.tblPersons  
    select person; 
if (!string.IsNullOrEmpty(FirstName))
    persons = from person in persons 
        where person.FirstName.StartsWith(FirstName) 
        select person;
if (!string.IsNullOrEmpty(MiddleName))
    persons = from person in persons 
        where person.MiddleName.StartsWith(MiddleName) 
        select person;
if (!string.IsNullOrEmpty(LastName))
    persons = from person in persons 
        where person.LastName.StartsWith(LastName) 
        select person;
if (!string.IsNullOrEmpty(SSN))
    persons = from person in persons 
        where person.SSN = SSN 
        select person;
if (BirthDate.HasValue)
    persons = from person in persons 
        where person.BirthDate == BirthDate.Value 
        select person;
return (from person in persons 
    join address in context.tblAddresses  
    on person.PersonID equals address.PersonID  
    orderby person.LastName        
    select new { person.PersonID, person.LastName,
        person.FirstName, person.SSN,  address.AddressLine1 })  
    .ToList()
    .CopyLinqToDataTable();
DRBlaise
+1  A: 

Might want to create an object to reflect a person, then add a filter method to it:

Person.AddFilter( fieldToLimit, operator, value)

This way you can add any number of filter criteria to the object.

Example:

Person.AddFilter( FirstName, Contains, "Bob"); Person.AddFilter( LastName, StartsWith, "Z");

Another way is to simply add your criteria to your Linq to SQL IQueryable datatype so that you can simply:

Person.Where( t => t.FirstName.Contains("Bob")).Where( t => t.LastName.StartsWith("Z"));

Dr. Zim