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, Strin开发者_Go百科g 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?
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 Person
s 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();
}
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.
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.
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.
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();
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"));
精彩评论