Problem
I have this WCF service providing search functionality – to be used by our client in his web application. There can be any number of search parameters and possible nesting of AND/OR operations.
Could you please have a look and share some ideas on if I can make it more client-friendly?
<con:Search>
<con:clientID>xxx</con:clientID>
<con:propertyQueries>
<wil:PropertyQuery Id="0">
<wil:PropertyID>DocumentType</wil:PropertyID>
<wil:Value>Statement</wil:Value>
<wil:Operator>EQUALS</wil:Operator>
</wil:PropertyQuery>
<wil:PropertyQuery Id="1" AND="0">
<wil:PropertyID>BANK</wil:PropertyID>
<wil:Value>HSBC</wil:Value>
<wil:Operator>EQUALS</wil:Operator>
</wil:PropertyQuery>
<wil:PropertyQuery Id="2" OR="1">
<wil:PropertyID>BANK</wil:PropertyID>
<wil:Value>BARCLAYS</wil:Value>
<wil:Operator>EQUALS</wil:Operator>
</wil:PropertyQuery>
<wil:PropertyQuery Id="3" AND="0">
<wil:PropertyID>Date</wil:PropertyID>
<wil:Value Start="1">11/01/2014</wil:Value>
<wil:Value End="1">11/02/2014</wil:Value>
<wil:Operator>BETWEEN</wil:Operator>
</wil:PropertyQuery>
</con:propertyQueries>
</con:Search>
The values in AND and OR attribute indicates the Id of PropertyQuery to which the condition to be added. For example:
<wil:PropertyQuery Id="2" OR="1">
performs an OR operation between Property Queries with ID of 1, 2.
Any better ideas out there to improve the interface?
Solution
The way you’re composing expressions seems very unintuitive to me. I think that you should take advantage of the fact that you’re trying to express (part of) the SQL syntax tree in the XML tree. So, AND should be a separate node and it should have its child expressions as child nodes. Something like (if I misunderstood how your AND/ORs work, that only proves my point):
<wil:combinedQuery operator="AND">
<wil:combinedQuery operator="AND">
<wil:PropertyQuery Id="0">
<wil:PropertyID>DocumentType</wil:PropertyID>
<wil:Value>Statement</wil:Value>
<wil:Operator>EQUALS</wil:Operator>
</wil:PropertyQuery>
<wil:PropertyQuery Id="3">
<wil:PropertyID>Date</wil:PropertyID>
<wil:Value Start="1">11/01/2014</wil:Value>
<wil:Value End="1">11/02/2014</wil:Value>
<wil:Operator>BETWEEN</wil:Operator>
</wil:PropertyQuery>
</wil:combinedQuery>
<wil:combinedQuery operator="OR">
<wil:PropertyQuery Id="1" AND="0">
<wil:PropertyID>BANK</wil:PropertyID>
<wil:Value>HSBC</wil:Value>
<wil:Operator>EQUALS</wil:Operator>
</wil:PropertyQuery>
<wil:PropertyQuery Id="2" OR="1">
<wil:PropertyID>BANK</wil:PropertyID>
<wil:Value>BARCLAYS</wil:Value>
<wil:Operator>EQUALS</wil:Operator>
</wil:PropertyQuery>
</wil:combinedQuery>
</wil:combinedQuery>
Dates should be in the unambiguous ISO 8601 format, not in the ambiguous and confusing (mm/dd/yyyy, or is it dd/mm/yyyy?) format.
I don’t like much the way you’re representing BETWEEN. I think that Start and End should be in elements (or attributes) with those names.
Without knowing its intended audience (Intranet / Internet), I’m assuming Intranet.
In my opinion, you are giving a lot of flexibility to this service in one method call, which isn’t immediately clear how a user would structure a call to this. The caller has to have knowledge of a “propertyId” which doesn’t make too much sense on a glance. I hope your API documentation describes this well, and how the user would retrieve what these IDs are.
So here is my assumption of the coded interface:
public interface ISearchService
{
List<object> Search(string clientId, IEnumerable<PropertyQuery> propertyQueries);
}
// Element
public class PropertyQuery
{
// Attribute
public string PropertyID { get; set; }
// Array
public IEnumerable<Value> Values { get; set; }
// Attribute
public byte AND { get; set; }
// Attribute
public byte OR { get; set; }
// Element
public string Operator { get; set; }
}
// Element
public class Value
{
// Attribute
public int Id { get; set; }
// Attribute
public byte Start { get; set; }
// Attribute
public byte End { get; set; }
// Text
public string Text { get; set; }
}
To make this a little simpler, I would add a single class that takes all parameters and a class returning the results, following the Request/Response pattern. This means that the interface can change over time, and should not affect backwards compatibility.
public interface ISearchService
{
// Gets the search criteria for the screen
SearchCriteriaResponse GetSearchCriterias(SearchCriteriaRequest request);
// Searches by the search criteria
SearchResponse SearchBy(SearchRequest searchCriteria);
}
// Base elements for a response
public abstract class ResponseBase
{
public bool Success { get; set; }
public string Message { get ; set; }
}
public class SearchCriteriaResponse : ResponseBase
{
public string[] DocumentTypes { get; set; }
public string[] BankName { get; set; }
}
public class SearchResponse : ResponseBase
{
public IEnumerable<SearchResult> Results { get; set; }
}
public class SearchRequest
{
public string ClientId { get; set; }
public string[] BankNamesToMatchExactly { get; set; }
public string[] BankNamesToPartiallyExactly { get; set; }
public string[] DocumentTypesToMatchExactly { get; set; }
public string[] DocumentTypesToPartiallyExactly { get; set; }
public DateTime? StartDate { get; set; }
public DateTime? EndDate { get;set; }
public bool CheckBetweenDates { get; set; }
}
This interface is a more clean on its approach. Its slightly larger, but it makes the ORing and ANDing much more obvious. It also makes the code cleaner at the service side as each of these properties have well-named properties and should be easy to implement:
public List<SearchResponse> SearchBy(SearchRequest searchCriteria)
{
IQueryable dbQuery = DbContext.BankNames;
foreach(var bankName in searchCriteria.BankNamesToMatchExactly)
{
dbQuery = dbQuery.Where(bn => bn.BankName == bankName);
}
foreach(var bankName in searchCriteria.BankNamesToMatchPartially)
{
dbQuery = dbQuery.Where(bn => bn.BankName.Contains(bankName);
}
// etc..
}