views:

234

answers:

7

I have converted my Datatable to json string use the following method...

public string GetJSONString(DataTable Dt)
{
    string[] StrDc = new string[Dt.Columns.Count];
    string HeadStr = string.Empty;
    for (int i = 0; i < Dt.Columns.Count; i++)
    {
        StrDc[i] = Dt.Columns[i].Caption;
        HeadStr += "\"" + StrDc[i] + "\" : \"" + StrDc[i] + i.ToString() + "¾" + "\",";
    }
    HeadStr = HeadStr.Substring(0, HeadStr.Length - 1);
    StringBuilder Sb = new StringBuilder();

    Sb.Append("{\"" + Dt.TableName + "\" : [");
    for (int i = 0; i < Dt.Rows.Count; i++)
    {
        string TempStr = HeadStr;
        Sb.Append("{");
        for (int j = 0; j < Dt.Columns.Count; j++)
        {
            if (Dt.Rows[i][j].ToString().Contains("'") == true)
            {
                Dt.Rows[i][j] = Dt.Rows[i][j].ToString().Replace("'", "");
            }
            TempStr = TempStr.Replace(Dt.Columns[j] + j.ToString() + "¾", Dt.Rows[i][j].ToString());
        }
        Sb.Append(TempStr + "},");
    }
    Sb = new StringBuilder(Sb.ToString().Substring(0, Sb.ToString().Length - 1));
    Sb.Append("]}");
    return Sb.ToString();
}

Is this fair enough or still there is margin for optimization to make it execute faster.... Any suggestion...

+4  A: 

There may well be ways of getting it to execute faster - but do you have any indication that you need it to execute faster? Do you have a good reason to believe this is a significant bottleneck in your code? If so, benchmark the code with some real data and profile the routine to work out where the time is going.

Jon Skeet
@Jon its really silly on my part to ask such a question before profiling it...
Pandiya Chendur
+1  A: 

You could tidy up some bits:

  1. Use string.Format() to avoid long x + y + z sequences. This may or may not make things faster (it would be marginal either way).
  2. You usually don't need .toString() when concatenating.

You could also pass in the StringBuffer to be populated, so that the caller might have the opportunity to bundle up several such operations into a single StringBuffer.

These suggestions are focused more on tidiness than performance, which I think should be the real focus unless this code is presenting as a bottleneck in your profiling.

Marcelo Cantos
+4  A: 

Before asking if you can optimise it to make it execute faster, the first question you need to ask yourself is, does it run fast enough for me? Premature optimisation is the curse of all of us (I know I've done it!). You could spend hours trying to micro-optimise this code, which might take it from taking, for example, 20ms to execute down to 15ms. Yes that'd be a reduction of 25%, but would 5ms really be worth 2 hours of your time? More importantly, would it provide enough of a benefit to your end users to warrant it?

Have you considered using the JsonSerializer from "Newtonsoft"? This may well be "quick enough", is fairly widely used and is thus more likely to be correct overall than anything I, or you, can write first time round.

Purely from a readability perspective (that may also allow the C# compiler / CLR to improve thing for you) you could consider changing long bits of string concatenation such as:

HeadStr += "\"" + StrDc[i] + "\" : \"" + StrDc[i] + i.ToString() + "¾" + "\",";

To:

HeadStr += string.Format("\"{0}\" : \"{0}{1}¾\",", strDc[i], i);

But for any changes you do make. Measure, Rinse, Repeat =)

Rob
@Rob as others suggested it would be better using a library?
Pandiya Chendur
@Pandiya, yes, almost always and certainly in this case if you're trying to write a Json Serialiser to use in production code. If, however, this is an exercise to gain understanding of how you'd write a serialiser, then carry right on =)
Rob
@Rob i certainly agree with you... Thanks for giving such an answer and comment...
Pandiya Chendur
@Pandiya - glad I could help :)
Rob
A: 

Why do you think it needs optimization? Is it really slow on some DataTables? I'd just serialize DataTable with something like newton JSON serializer, if it's serializable at all.

hoodoos
It's a DataTable. nuff said. If you cannot serialize a data table, umm.. well there is always golf.
Sky Sanders
A: 

Refactor your code, use a tool like ReSharper, JustCode etc to tidy it up a bit. Extract methods and use individual tests ( Test Driven Development-ish ) to find bottlenecks in your code and then tweak those.

But your first step should be: Refactor!

Filip Ekberg
A: 

I would suggest a different solution,if you are using .net 3.0 or 3.5

instead of doing this

  1. Convert datatable into xml
  2. use xmlserializer to convert the xml to your domain object
  3. Using JavaScriptSerializer(System.Web.Extensions.dll) to serialize the domain object to json string.

cheers

Ramesh Vel
care to give the reason..?
Ramesh Vel
A: 

The problem with the code isn't speed, but that it's not cleaned up. I've done some clean-up, but you could probably do even more:

public string GetJSONString2(DataTable table)
{
    StringBuilder headStrBuilder = new StringBuilder(table.Columns.Count * 5); //pre-allocate some space, default is 16 bytes
    for (int i = 0; i < table.Columns.Count; i++)
    {
        headStrBuilder.AppendFormat("\"{0}\" : \"{0}{1}¾\",", table.Columns[i].Caption, i);
    }
    headStrBuilder.Remove(headStrBuilder.Length - 1, 1); // trim away last ,

    StringBuilder sb = new StringBuilder(table.Rows.Count * 5); //pre-allocate some space
    sb.Append("{\"");
    sb.Append(table.TableName);
    sb.Append("\" : [");
    for (int i = 0; i < table.Rows.Count; i++)
    {
        string tempStr = headStrBuilder.ToString();
        sb.Append("{");
        for (int j = 0; j < table.Columns.Count; j++)
        {
            table.Rows[i][j] = table.Rows[i][j].ToString().Replace("'", "");
            tempStr = tempStr.Replace(table.Columns[j] + j.ToString() + "¾", table.Rows[i][j].ToString());
        }
        sb.Append(tempStr + "},");
    }
    sb.Remove(sb.Length - 1, 1); // trim last ,
    sb.Append("]}");
    return sb.ToString();
}
Mikael Svenson
@mikeal what is the use of pre allocating?
Pandiya Chendur
@pandiya You allocate space in the stringbuilder for the data you add to it. A StringBuilder has underlying storage, which default is 16 bytes. When this is exceeded it will allocate a bigger buffer, and copy the old contents to the new buffer. By pre-allocation a buffer of the expected end size, it will not have to re-allocate a new buffer as you add more and more data. It will make the code a bit faster as you allocate fewer objects. Same goes for List's as well.
Mikael Svenson
@mikeal svenson thanks for this comment....
Pandiya Chendur