From object to proper types: Is this the/a "right" way to work with this non-generified, object-returning API?

Question

One particular aspect of some code I've written is causing me minor headaches, I can't pinpoint the reason - all the type checking and casting is making me feel uneasy. The code works as it is right now. I'd like to know wether or not there's a better way to handle the type-specific aspects of the code.

I'm using a non-generified, object-returning JSON parser which makes me go through various incantations in my generified code.

The signature of the parse method is public static object JsonDecode(string json) The runtime type of the object it returns can be ArrayList, Hashtable, double or string. I'm calling the JsonDecode method on a Twitter Search response, which returns the result tweets as a top level object of the form:

{"results":[
 {"text":"@twitterapi  http:\/\/tinyurl.com\/ctrefg",
 "to_user_id":396524,
 "to_user":"TwitterAPI",
 "from_user":"jkoum",
 "metadata":
 {
  "result_type":"popular",
  "recent_retweets": 109

 }, ... MORE-DATA ...}

The context in which I'm using the JsonDecode(string json) method is

private IList<Tweet> searchResult = new List<Tweet>();
var jsonDecoded = JSON.JsonDecode(responseString);

IList rawTweets = 
    (IList)((Hashtable)jsonDecoded)["results"];

foreach (var rawTweet in rawTweets) 
{
    searchResult.Add(new Tweet((Hashtable) rawTweet));
}

The Tweet class does its own type checking and casting

 class Tweet : DynamicObject
{
    private IDictionary<string, string> stringValues = 
        new Dictionary<string, string>();
    private IDictionary<string, double> numberValues =
        new Dictionary<string, double>();

    public Tweet(Hashtable rawTweet) 
    {
        FlattenJSON(rawTweet);
    }

    //flatten input and assign to correct map/dictionary based on JSON value type
    private void FlattenJSON(Hashtable table)
    {
        foreach (DictionaryEntry entry in table)
        {
            //this code is not handling the case, that the Tweet contains a JSON array
            //not necessary as of now: this code is intended for demo purposes in a talk
            //I'm giving on Friday 2010-06-25
            if (entry.Value is String)
                stringValues.Add((string)entry.Key, (string)entry.Value);
            else if (entry.Value is Double)
                numberValues.Add((string)entry.Key, (double)entry.Value);
            else if (entry.Value is Hashtable)
                FlattenJSON((Hashtable)entry.Value);
        }
    }
    ...
}

Am I handling the type checks in the FlattenJSON method correctly? What about the casts in the code snippet building the IList and constructing the searchResult IList<Tweet>? Would you have written the code in a different way?

As a side note, the complete code is available via http://github.com/codesurgeon/pop-tweets The code is intended for a demo in a talk that I'll be giving on selected .NET features. The Tweet class is a DynamicObject and it overrides TryGetMember, see full listing on github.

Thank you ;)

P.S.: [FYI] This is a more specific version of my previous post, requesting a general code review http://stackoverflow.com/questions/3113293/how-to-be-a-good-c-citizen-review-requested-for-c-4-0-dynamic-sample-code

Answer 1

A few things stand out to me:

First, you should so some argument validation. Check to see if "table" is null before you start using it. Otherwise, you will get an unhandled NullReferenceException and it will confuse your consumer.

Next, does this actually work? The "is" keyword (I thought) only worked with reference types, and double is a value type. I would assume that this wouldn't work and you'd need to instead do a double.TryParse().

Lastly, there is a lot of casting on-the-fly. When you are interacting with a 3rd-party API, I would argue that you should have an interface where you convert from "their weird stuff" to "your clean stuff". So even if the API is messy and difficult, clean it up and present it to your consumer in the proper way. Don't propogate the bad design. In other words, as a consuming developer, I would want some obvious and clean data structures to work with. Like: foreach(Tweet tweet in Twitter.GetTweets()) or something. I don't care that you happen to use json right now. That implementation should be be invisible to me.

Hope that helps...

Answer 2

I would not do it like that.

First of all, your tweet class is not a real object, it's a data container. You could just continue to work with the jsonDecoded variable, since your tweet class doesn't add any functionality or abstraction (other than abstracting away the JSON decoder).

How I would do it:

1. Create POCOs which holds the information: Tweet and a Metadata class.
2. Create a serializer which takes the JSON string and returns the Tweet class

By dividing it into two steps and also creating real classes you will have it much easier in the future. Please read about Single Responsibility.