3
\$\begingroup\$

I have a custom object that is based in the following models:

public class Filter<T> where T : new()
{
    public T Object { get; set; }
    public int Page { get; set; }
    public int ItemsPerPage { get; set; }
}

public class TransactionFilter<T> : Filter<T> where T : new()
{
    public DateTime InitialDate { get; set; }
    public DateTime EndDate { get; set; }
    public DateTime InitialPayment { get; set; }
    public DateTime EndPayment { get; set; }
    public List<string> Filters { get; set; }
}

public class Transaction<T> : Base where T : new()
{  
    public T PaymentObject { get; set; }
}

public class Base
{
    public bool Sandbox { get; set; }
    public PaymentMethod PaymentMethod { get; set; }
}

public class PaymentMethod
{
    public string Code { get; set; }
}

And is built on:

var queryObj = new TransactionFilter<Transaction<object>>()
{
    Object = new Transaction<object> { PaymentMethod = new PaymentMethod { Code = "1" }, Sandbox = false },
    InitialDate = new DateTime(2019, 03, 01),
    EndDate = new DateTime(2019, 04, 01),
    InitialPayment = new DateTime(2019, 03, 01),
    EndPayment = new DateTime(2019, 04, 01),
    Filters = new List<string>() { "ID", "Customer", "Reference", "PaymentMethod", "Application", "Ammount", "Vendor", "Status", "PaymentDate", "CreatedDate" }
};

And should be transformed in a querystring like this:

?Filter=ID&Filter=Customer&Filter=Reference&Filter=PaymentMethodCode&Filter=Application&Filter=Amount&Filter=Vendor&Filter=Status&Filter=PaymentDate&Filter=CreatedDate&Filter=PaymentMethod&Page1=PaymentMethod&Page=1&ItensPerPage100&Object.Sandbox=False&PaymentMethod.Code=1

Yes, Filter property is correct, the API needs to receive it this way... and that was my big issue to find a way to properly mount as needed based on the models.

Based on a lot of research, questions, debugging, exceptions and a lot of coffee, I finally got this as a result to build it as it should be:

private string QueryString(object request, string propertyName = null)
{
    if (request == null) throw new ArgumentNullException(nameof(request));

    var queryString = new StringBuilder();

    var properties = request.GetType().GetProperties()
        .Where(x => x.CanRead)
        .Where(x => x.GetValue(request, null) != null)
        .Where(x => !x.PropertyType.IsClass || x.PropertyType.IsClass && x.PropertyType.FullName == "System.String")
        .ToDictionary(x => x.Name, x => x.GetValue(request, null));

    foreach (var (key, value) in properties)
    {
        if (string.IsNullOrEmpty(propertyName))
            queryString.AppendFormat("{0}={1}", Uri.EscapeDataString(key), Uri.EscapeDataString(value.ToString()));
        else
            queryString.AppendFormat("{0}.{1}={2}", Uri.EscapeDataString(propertyName), Uri.EscapeDataString(key), Uri.EscapeDataString(value.ToString()));
        queryString.AppendFormat("&");
    }

    var classTypes = request.GetType().GetProperties()
        .Where(x => x.CanRead)
        .Where(x => x.GetValue(request, null) != null && x.PropertyType.IsClass && x.PropertyType.FullName != "System.String" && !(x.GetValue(request, null) is IEnumerable))
        .ToDictionary(x => x.Name, x => x.GetValue(request, null));

    var collectionTypes = request.GetType().GetProperties()
        .Where(x => x.CanRead)
        .Where(x => x.GetValue(request, null) != null)
        .ToDictionary(x => x.Name, x => x.GetValue(request, null))
        .Where(x => !(x.Value is string) && x.Value is IEnumerable)
        .ToDictionary(x => x.Key, x => x.Value);

    foreach (var (key, value) in collectionTypes)
    {
        var valueType = value.GetType();
        var valueElemType = valueType.IsGenericType
                                ? valueType.GetGenericArguments()[0]
                                : valueType.GetElementType();

        if (valueElemType.IsPrimitive || valueElemType == typeof(string))
        {
            if (!(value is IEnumerable enumerable)) continue;
            foreach (var obj in enumerable)
            {
                if (string.IsNullOrEmpty(propertyName))
                    queryString.AppendFormat("{0}={1}", Uri.EscapeDataString(key), Uri.EscapeDataString(obj.ToString()));
                else
                    queryString.AppendFormat("{0}.{1}={2}", Uri.EscapeDataString(propertyName), Uri.EscapeDataString(key), Uri.EscapeDataString(obj.ToString()));

                queryString.AppendFormat("&");
            }
        }
        else if (valueElemType.IsClass) 
        {
            var count = 0;
            foreach (var className in (IEnumerable) value)
            {
                var queryKey = $"{key}[{count}]";
                queryString.AppendFormat(QueryString(className, queryKey));
                count++;
            }
        }
    }
    foreach (var (key, value) in classTypes)
        queryString.AppendFormat(QueryString(value, key));

    return "?" + queryString;
}

It's called in a quite simple way to mount the query and combine with the URL on API request:

var query = QueryString(queryObj);
var response = await client.GetAsync("https://api_address.com/Transaction/Get" + query);

It works as expected and provides the desired result, which is good enough to me, but I believe that it could be achieved on a simple/better way, so I would really appreciate to see a different view about that, if that could be considered as a good approach for this situation, both function and the call for it, to achieve the expected result.

Thanks!

\$\endgroup\$
  • \$\begingroup\$ Filters should be a HashSet<string> instead of a List, because it doesn't make sense that it may contain duplicates. \$\endgroup\$ – Brad M May 2 at 13:10
  • 3
    \$\begingroup\$ You should check out docs.microsoft.com/en-us/dotnet/api/… as it has example to walk object tree and catch for recursion. It's not hard to change it to out put nodes with parents to build a tree view. Which then can be used to build your query string. \$\endgroup\$ – CharlesNRice May 2 at 15:02
  • \$\begingroup\$ You mentioned this code works for your particular scenario. But do you intend to use it for other filters as well? Do you expect your filter classes to change over time? \$\endgroup\$ – dfhwze Jun 3 at 19:45
3
\$\begingroup\$
private string QueryString(object request, string propertyName = null)
{
  if (request == null) throw new ArgumentNullException(nameof(request));

It is fine to check if a parameter is null, but because you call this method recursively, the request parameter may be a property that is null on an object that is otherwise valid:

class ParentObject
{
  ChildObject Child { get; set; }
  int PrimitiveValue { get; set; }
}

class ChildObject
{
  string Name { get; set; }
}


ParentObject po = new { ChildObject = null, PrimitiveValue = 10 }

Are you sure this situation should throw an exception, when handling ChildObject recursively?


You are not dealing with struct types (value types that are not primitives). They are handled as primitive types, which goes well for DateTime, but what about structs with two or more useful properties?


You should be aware of the possibility to go into an infinite recursion if a type have a property of its own kind: a DateTime has the property Date which is of type DateTime, so that is a candidate for an infinite recursion.


As CharlesNRice writes in this comment, you should check that each object is only handled once.


return "?" + queryString;

Because you call the method recursively, you end up with '?' more places than just at the beginning. Is that intentionally?


PaymentMethod.Code=1

When running your example the above is generated. But shouldn't that be Object.PaymentMethod.Code=1 ?


All in all I think you do too much in the same method, and you query the request for properties unnecessarily many times. I think I would try with an approach like:

  IEnumerable<(string key, object value)> CollectProperties(object request, string prefix = null)
  {
    List<(string key, object value)> properties = new List<(string key, object value)>();

    if (IsHandled(request)) return properties;

    prefix = prefix == null ? "" : $"{prefix}.";

    foreach (PropertyInfo pi in request.GetType().GetProperties().Where(p => p.CanRead))
    {
      string propertyPrefix = $"{prefix}{pi.Name}";
      if (IsPrimitive(pi))
        properties.Add(HandlePrimitive(..., propertyPrefix));
      else if (IsEnumerable(pi))
        properties.AddRange(HandleEnumerable(..., propertyPrefix));
      else if (IsClass(pi))
        properties.AddRange(HandleClass(..., propertyPrefix));
      else if (IsStruct(pi))
        properties.AddRange(HandleStruct(..., propertyPrefix));
    }

    return properties; 
  }

Each of the Handle<...>(...) methods may call the above method recursively. The caller handles the format and url encoding and finally the concatenation to one string using string.Join("&", properties.Select(format...)). In this way you only have one place where the format occur.

\$\endgroup\$
  • 2
    \$\begingroup\$ Note that .GetProperties().Where(p => p.CanRead) includes indexers. If you don't want to include them, add .Where(p => p.GetIndexParameters().Length == 0). Furthermore, I like the approach for visiting by type of object, but this is not a sinecure. There are nullables, dynamic classes, anonymous classes, tuples, generic collection types, and so on.. This approach requires alot of attention. \$\endgroup\$ – dfhwze Jun 3 at 19:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.