9
\$\begingroup\$

I have to interface with a horrible PHP SOAP API that returns nested arrays of strings. I've written a helper that can parse the SOAP response and return a dynamic object but I need at least some properties to be strongly typed.

The API returns a lot more data that I need to keep up front (but need to interrogate later) and apart from a few guaranteed fields, it is truly dynamic (it could contain pretty much anything).

I have created a proxy class that wraps the dynamic object like so:

public class Basket
{
    public Basket(string basketResponse, BasketToken token)
    {
        this.Token = token;
        this.BasketResponse = new SoapSimplifier(basketResponse).ToJObject();
    }

    public string FirstName { get { return this.BasketResponse.deliverydata.firstname; } }
    public string LastName { get { return this.BasketResponse.deliverydata.lastname; } }
    public string Tel { get { return this.BasketResponse.deliverydata.mobile; } }
    public string Email { get { return this.BasketResponse.deliverydata.email; } }
    public string Address1 { get { return this.BasketResponse.deliverydata.add1; } }
    public string PostCode { get { return this.BasketResponse.deliverydata.postcode; } }

    public BasketToken Token { get; }

    public dynamic BasketResponse { get; }
}

The BasketResponse contains more data that I will to interrogate later on (with a shed-load of TryParse,Try/Catch and Linq) so I do need to keep it. I also need to store it in a database - fortunately Cosmos DB can handle dynamics.

My questions are:

  1. Is this a sensible approach, using the dynamic object like one might use backing fields?
  2. Would it be better to interrogate the dynamic BasketResponse in the constructor and set the values of the properties when basket is instantiated?

I'm especially concerned about performance.


May be relevant:

Here is the SoapSimplifier

    internal class SoapSimplifier
{
    private static XNamespace Xsi = "http://www.w3.org/2001/XMLSchema-instance";
    private XElement ParsedResponse { get; set; }
    private JObject ParsedJObject { get; set; }

    public SoapSimplifier(string responseString)
    {
        this.ParsedResponse = XDocument.Parse(responseString).Descendants("return").First();
    }

    public JObject ToJObject()
    {
        if(this.ParsedJObject == null)
        {
            ParsedJObject = this.XElementToJObject(ParsedResponse);
        }
        return ParsedJObject;
    }

    public override string ToString()
    {
        return this.ToJObject().ToString();
    }

    private JObject XElementToJObject(XElement Item)
    {
        JObject jo = new JObject();
        foreach (var i in Item.Elements("item"))
        {
            var valNode = i.Element("value");
            if (valNode != null)
            {
                var nodeType = valNode.Attribute(Xsi + "type").Value;
                if (nodeType == "xsd:string")
                {
                    jo.Add(new JProperty(i.Element("key").Value, i.Element("value").Value));
                }
                else if (nodeType == "ns2:Map")
                {
                    jo.Add(new JProperty(i.Element("key").Value, XElementToJObject(valNode)));
                }
                else if (nodeType == "SOAP-ENC:Array")
                {
                    JArray ja = new JArray();
                    foreach (var node in valNode.Elements())
                    {
                        ja.Add(XElementToJObject(node));
                    }
                    jo.Add(new JProperty(i.Element("key").Value, ja));
                }
            }

        }
        return jo;
    }
}

This takes the SOAP response generated from a PHP array sent via SOAP (similar to this one) and parses it into a JObject.

This is SOAP Jim, but not as we know it. The API provider will not supply a WSDL. Apart from a few guaranteed keys the array(s) returned could contain any number of key/value pairs and or other arrays.

As PHP arrays are dynamic, I can't think of a way of representing them in C# that isn't also dynamic.

I'm using jObject because it's fast and I can simultaneously create a readable JSON string for logging and a dynamic object that can be queried in c# with dot syntax.

\$\endgroup\$
  • 2
    \$\begingroup\$ I'm struggling to find a reason for using dynamic here. Can you post the code from the SoapSimplifier constructor? Maybe I'm missing something about what exactly it's trying to achieve. \$\endgroup\$ – Flater Oct 31 '17 at 16:21
  • \$\begingroup\$ Well @Flater, you asked for it. I said it was horrible! \$\endgroup\$ – Jason Elkin Oct 31 '17 at 21:18
4
\$\begingroup\$
        this.BasketResponse = new SoapSimplifier(basketResponse).ToJObject();

At this point you know you have a JObject: by storing it in a dynamic property you're losing information. I would say there's a case to at least consider storing it in a typed backing field.


    public string FirstName { get { return this.BasketResponse.deliverydata.firstname; } }
    public string LastName { get { return this.BasketResponse.deliverydata.lastname; } }
    public string Tel { get { return this.BasketResponse.deliverydata.mobile; } }
    public string Email { get { return this.BasketResponse.deliverydata.email; } }
    public string Address1 { get { return this.BasketResponse.deliverydata.add1; } }
    public string PostCode { get { return this.BasketResponse.deliverydata.postcode; } }

Given that this section of data is supposed guaranteed, IMO it makes sense to parse it to a DeliveryData object and then only store the dynamic part of the message as dynamic.

But even if you prefer to go dynamic all the way, this code is very WET. Since there's no compile-time checking of names, I would consider it obligatory to at least factor out BasketResponse.deliverydata as a private property.


            if (valNode != null)
            {
                var nodeType = valNode.Attribute(Xsi + "type").Value;
                if (nodeType == "xsd:string")
                {
                    jo.Add(new JProperty(i.Element("key").Value, i.Element("value").Value));
                }
                else if (nodeType == "ns2:Map")
                {
                    jo.Add(new JProperty(i.Element("key").Value, XElementToJObject(valNode)));
                }
                else if (nodeType == "SOAP-ENC:Array")
                {
                    JArray ja = new JArray();
                    foreach (var node in valNode.Elements())
                    {
                        ja.Add(XElementToJObject(node));
                    }
                    jo.Add(new JProperty(i.Element("key").Value, ja));
                }
            }

A switch (nodeType) might be clearer. It would also emphasise the lack of handling of the default case. Surely you want to at least log a warning if the node is none of the above?

\$\endgroup\$
2
\$\begingroup\$

The answer to your second question...

Would it be better to interrogate the dynamic BasketResponse in the constructor and set the values of the properties when basket is instantiated?

Is yes.

You're fetching the data, so set it in the constructor, that way, you're doing the work in one place.

Is there a need to use the BasketResponse Property again. Aren't you mapping all the data you need already?

\$\endgroup\$
  • \$\begingroup\$ Yes, I need to keep hold of BasketResponse. My instinct was to set values in the constructor but that means that I am duping the data. My (limited) understanding of dynamics suggests that this would also force the compiler to evaluate the dynamic objects/properties when the class is constructed, whether or not a property is actually used. Wrapping them in the getter makes them lazy which seems more efficient. \$\endgroup\$ – Jason Elkin Nov 1 '17 at 8:55

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.