I wrote some code a few weeks ago, to take a string similar toseq[]=8&seq[]=7&seq[]=5&seq[]=1&seq[]=4&seq[]=3&seq[]=6&seq[]=0&seq[]=2and put it into a gallery, to be ordered by jQuery UI's Sortable, which works all good, however, I'm concerned about the amount of times I .Replace the values in the string.This code has been bugging me, because I'm employed to rewrite unethical code, and the few times I actually write code from scratch, it looks like this. So, basically, I would love your opinions on this code.To clarify, the imgMgr is to get the JSON string, and the nodelist is for getting an XML doc of images from the imgMgr.#region BuildGalleryprivate void BuildGallery(){    StringBuilder sb = new StringBuilder();    sb.Append(<div class=\controls\><center><button class=\js-serialize\ style=\border: 0;height: 25px;background-color: #F4FA58;margin-bottom: 8px;\>Save Layout</button></center></div><div class=\sortable\><ul id=\sortable\>);    int pos = 0;    string str = ;    ImageManagerV1 imgMgr = new ImageManagerV1();    str = imgMgr.galleryOrder(LocalConfig.ImageManagerAuthCode, Convert.ToInt32(hdnListingId.Value), );  //  str = seq[]=8&seq[]=7&seq[]=5&seq[]=1&seq[]=4&seq[]=3&seq[]=6&seq[]=0&seq[]=2;    str.Replace(seq[]=,-).Split('-');    XmlNodeList nodelist = xmlDoc.SelectNodes(image);    str = str.Replace(&, );    if (str == ) {        for (int i = 0; i < nodelist.Count; i++) {            str += seq[]= + i;        }    }    string[] strNew = str.Replace(seq[]=, -).Split('-');        str = ;        for (int i = 0; i < strNew.Length; i++) {            if (strNew[i] != ) {                str += - + strNew[i];            }        }    if (nodelist.Count > strNew.Length) {        for (int f = strNew.Length; f < nodelist.Count; f++) {            str += - + f;        }    }    int val2 = 0;    string[] stringArray = str.Split('-');    foreach (string num in stringArray) {        var newNum = num.ToString();        if (newNum.Length > 0) {            val2 = Convert.ToInt32(newNum);            XmlNode image = nodelist[val2];            string thumbpath = image.Attributes[thumb].Value;            string detailpath = image.Attributes[detail].Value;            string fileName = detailpath.Substring(detailpath.LastIndexOf(@/) + 1);            sb.AppendFormat(<li class=\thumb ui-state-default\ id=\seq_{0}\>, val2);            sb.AppendFormat(<img src=\{0}\ alt=\{2}\ title=\Gallery: {2}\ width=\90\ height=\90\/><a href=\\ data=\{4}\class=\makeHero\>Make Hero</a><a href=\\ data=\{4}\class=\deleteImage\>Del</a>, thumbpath, detailpath, listing.ListingName, pos, fileName);            sb.Append(</li>);        }    }    sb.Append(</ul><div class=\clear\></div></div>);        if (sb.Length > 0)        {            litGallery.Text = string.Format(<div id=\listing-gallery\ class=\listing-gallery\>{0}</div>, sb.ToString());        }    }#endregion
Ordering Gallery images by custom order
c#;strings;array;image
string str = ;There's no sense in assigning str to an empty string just to over write it two lines later. This would be much more terse. ImageManagerV1 imgMgr = new ImageManagerV1();string str = imgMgr.galleryOrder(LocalConfig.ImageManagerAuthCode, Convert.ToInt32(hdnListingId.Value), );str.Replace(seq[]=,-).Split('-');Why not just split on seq[]=? The Replace feels unnecessary here. Actually, the more I look at this, why are you splitting? Split returns an array, but you're not making an assignment here. I really think you need to decide what this line is supposed to do. str = str.Replace(&, );if (str == ) {    for (int i = 0; i < nodelist.Count; i++) {        str += seq[]= + i;    }}Two things. As a maintainer of the code, I've no idea why you're stripping out the ampersands. A comment here is in order. Secondly, this loop needs a StringBuilder. StringBuilder is a mutable string of characters and thus, more efficient in this situation. string[] strNew = str.Replace(seq[]=, -).Split('-');Okay, now you can skip the Replace and split directly on seq[]=. Speaking of, that string seems to show up an awful lot. You should replace it with a constant. string[] separators = new string[] {seq[]=};strNew = str.Split(separators, SplitStringOptions.None);    for (int i = 0; i < strNew.Length; i++) {        if (strNew[i] != ) {            str += - + strNew[i];At first glance, I want to tell you to use a string builder again, but I honestly don't have the foggiest idea why you just split the string apart, just to put it back together. Forget what I just said about splitting, and just Replace(seq[]=,-). Don't split. It looks to be unecessary. 