Simplifying a web service method

Question

I have the following method in a web service class. I'm a little unhappy about the big block of new JProperty(...) calls in the for loop. Is there a way to simplify that?

public string UserCatalog(string numericSessionId, JObject incomingRequestJson)
{
        JObject json = new JObject();
        JObject returningJson = new JObject();
        JArray userCatalogArray = new JArray();

        string deviceId = incomingRequestJson.SelectToken("deviceId", true).ToString();
        string version = getOptionalData(incomingRequestJson, "version", "1.0.0");
        requireMinVersion(incomingRequestJson);

        IEnumerable<Bookcard> catalog = readerBLL.UserCatalog(numericSessionId, deviceId);
        var publicKeyRSA = readerTools.GetPublicKey(HttpUtility.HtmlDecode(numericSessionId), HttpUtility.HtmlDecode(version));

        //Build the userCatalog json array
        foreach (Bookcard card in catalog)
        {
            JObject arrayEntry = new JObject(
                new JProperty("bookThumbnailUrl", card.bookThumbnailUrl),
                new JProperty("bookId", card.bookId),
                new JProperty("bookTitle", card.bookTitle),
                new JProperty("titlePrefix", card.titlePrefix),
                new JProperty("author", card.author),
                new JProperty("annotation", card.annotation),
                new JProperty("publisher", card.publisher),
                new JProperty("numPages", card.numPages),
                new JProperty("returnDate", card.returnDate),
                new JProperty("downloaded", card.downloaded),
                new JProperty("deviceId", card.deviceId),
                new JProperty("currentPageLabel", card.currentPageLabel),
                new JProperty("furthestPageLabel", card.furthestPageLabel),
                new JProperty("currentReadPosition", card.currentReadPosition),
                new JProperty("furthestReadPosition", card.furthestReadPosition),
                new JProperty("lastReadTimestamp", card.lastReadTimestamp),
                new JProperty("bookLength", card.bookLength),
                new JProperty("ttsEnabled", card.ttsEnabled),
                new JProperty("mackinCheckoutID", card.mackinCheckoutID),
                new JProperty("runtime", card.runtime),
                new JProperty("readerType", card.readerType),
                new JProperty("dop", card.dop),
                new JProperty("externalId", card.externalId),
                new JProperty("externalSessionKey", card.externalSessionKey),
                new JProperty("externalCheckoutSessionKey", card.externalBookSessionKey),
                new JProperty("externalCheckoutId", card.externalCheckoutId),
                new JProperty("externalAccountId", card.externalAccountId + findawayBLL.AccountSuffix)
                );
            if (card.printAllowed && !readerTools.isOldVersion(1, 3, 0, version))
            {
                arrayEntry.Add(new JProperty("printAllowed", readerTools.EncryptRSA("print_OK_" + card.bookId, publicKeyRSA)));
            }
            userCatalogArray.Add(arrayEntry);
        }

        returningJson.Add("userCatalog", userCatalogArray);
        return returningJson.ToString();
    }

Answer 1

As @WillNewton commented, you could always move all that newing up elsewhere (perhaps in an extension method that extends the Bookcard type and adds a ToJObject method), but the length would remain the same - there's not much repetition in here.

If all properties of BookCard (that's not a typo - I really think it should be BookCard) are mapped to a JProperty, then you could use some reflection to get each property's name and value:

public static IEnumerable<JProperty> GetJProperties(this BookCard card)
{
    var create = new Func<string, object,JProperty>((name, value) => new JProperty(name, value));
    foreach (var property in card.GetType().GetProperties())
    {
        yield return create(property.Name, property.GetValue(card));
    }
}

and then I presume (haven't tested) this would work:

public static JObject ToJObject(this BookCard card)
{
    return new JObject(card.GetJProperties());
}

This extension method would allow you to rewrite your foreach loop like this:

foreach(BookCard card in catalog)
{
    var arrayEntry = card.ToJObject();
    // ...
}

Alternatively, JSON.net gives you the JsonConvert class; you might be interested in the Newtonsoft.Json.JsonConvert.SerializeObject and Newtonsoft.Json.JsonConvert.DeserializeObject static methods, each with overloads that let you customize the process.

Answer 2

It is not clear (since you don't show the definition of card) however if all you are doing is creating properties for each public property of card then try this:

JObject arrayEntry = JObject.FromObject(card);

You might need a few more lines to add or remove additional properties based on your business rules. But this would cut down on a lot of the clutter.

Answer 3

If you want to remove the big block of new JProperties, then you need to abstract what varies and find a way in which you can loop over the list of properties you wish to serialise to JSON. You can do this using reflection, which won't be as fast as this code, but it would be more generic and reusable. The XmlSerializer that's been in the .net Framework since version 1.0 works this way, and I'd recommend a similar approach using attributes on public properties and fields that you wish to serialize.

I'm not particularly experienced in serialising JSON, but I'd be suprised if there weren't already a plethora of libraries using this approach. In fact a quick Google turns up the JSON.net library which looks popular and appears to work in this manner.