Ordering Gallery images by custom order

I wrote some code a few weeks ago, to take a string similar to


seq[]=8&seq[]=7&seq[]=5&seq[]=1&seq[]=4&seq[]=3&seq[]=6&seq[]=0&seq[]=2

and 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 BuildGallery
private void BuildGallery()
{

    StringBuilder sb = new StringBuilder();

    sb.Append("Save Layout");
    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  strNew.Length) {
        for (int f = strNew.Length; f  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("", val2);
            sb.AppendFormat("Make HeroDel", thumbpath, detailpath, listing.ListingName, pos, fileName);
            sb.Append("");
        }
    }
    sb.Append("");
        if (sb.Length > 0)
        {
            litGallery.Text = string.Format("{0}", sb.ToString());
        }
    }
#endregion

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.

StackExchange Code Review Q#75760, answer score: 3