CODEWITHSUNDEEP
c#stringbuilder
vix
vix

Reputation: 11

Effective use of StringBuilder

I need to append an Sqlquery for each row in a datatable.I have 36 columns and based on the datatype of each column i need to append sqlquery.Can anyone suggest me the effective way to do it.Is it bad way of coding to use " + " operator to append text in between the append ?

Following is my code.

 query ="INSERT INTO MASTERPI (tag,instrumenttag)");
 query += "VALUES ('" + createTagRow["tag"].ToString() + "','" + createTagRow["instrumenttag"].ToString() + "'");

Thanks, Vix

Upvotes: 1

Views: 1131

Answers (3)

Oliver
Oliver

Reputation: 45101

As already mentioned you should consider two possibilities:

  1. Aaronaught: Parameterize that query
  2. If you need full control over the statement string, you should take a look into .FormatWith extension and refactor your different blocks into classes, which give the needed parts back.

A little example:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
using System.Web.UI;

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            var columns = new CommaSeparated();

            columns.Add("tag");
            columns.Add("instrumenttag");
            columns.Add("pointsource");
            columns.Add("pointtype");
            columns.Add("dataowner");
            columns.Add("dataaccess");
            columns.Add("location1");
            columns.Add("location2");
            columns.Add("location3");
            columns.Add("location4");
            columns.Add("location5");
            columns.Add("...");


            var values = new CommaSeparated();

            values.EncloseEachElementWith = "'";
            values.Add(createTagRow["tag"].ToString());
            values.Add(createTagRow["instrumenttag"].ToString());
            values.Add(createTagRow["pointsource"].ToString());
            values.Add(createTagRow["pointtype"].ToString());
            values.Add(createTagRow["dataowner"].ToString());
            values.Add(createTagRow["dataaccess"].ToString());
            values.Add(createTagRow["location1"].ToString());
            values.Add(createTagRow["location2"].ToString());
            values.Add(createTagRow["location3"].ToString());
            values.Add(createTagRow["location4"].ToString());
            values.Add(createTagRow["location5"].ToString());
            values.Add(createTagRow["..."].ToString());

            //INSERT INTO MASTERPI ({columns}) values ({values})
            var query = "INSERT INTO MASTERPI ({columns}) values ({values})".FormatWith(new { columns, values });

            Console.WriteLine(query);
            Console.ReadKey();
        }
    }

    public class CommaSeparated : List<string>
    {
        public CommaSeparated()
            : base()
        {
            EncloseEachElementWith = String.Empty;
        }

        public override string ToString()
        {
            var elements = this.Select(element => String.Format("{0}{1}{0}", EncloseEachElementWith, element));

            return String.Join(", ", elements.ToArray());
        }

        public string EncloseEachElementWith { get; set; }
    }

    public static class StringExtensions
    {
        public static string FormatWith(this string format, object source)
        {
            return FormatWith(format, null, source);
        }

        public static string FormatWith(this string format, IFormatProvider provider, object source)
        {
            if (format == null)
                throw new ArgumentNullException("format");

            Regex r = new Regex(@"(?<start>\{)+(?<property>[\w\.\[\]]+)(?<format>:[^}]+)?(?<end>\})+",
              RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);

            List<object> values = new List<object>();
            string rewrittenFormat = r.Replace(format, delegate(Match m)
            {
                Group startGroup = m.Groups["start"];
                Group propertyGroup = m.Groups["property"];
                Group formatGroup = m.Groups["format"];
                Group endGroup = m.Groups["end"];

                values.Add((propertyGroup.Value == "0")
                  ? source
                  : DataBinder.Eval(source, propertyGroup.Value));

                return new string('{', startGroup.Captures.Count) + (values.Count - 1) + formatGroup.Value
                  + new string('}', endGroup.Captures.Count);
            });

            return string.Format(provider, rewrittenFormat, values.ToArray());
        }
    }
}

Upvotes: 0

Aaronaught
Aaronaught

Reputation: 122644

If you're already using a StringBuilder then it's detrimental to use regular string concatenation at the same time. You're negating half of the utility of the StringBuilder class. Don't do it. Use the StringBuilder.Append method exclusive and get rid of those + statements.

I'm also thinking that all those createTagRow(...).ToString() calls are wasteful. Something in there is doing the work of serializing those elements to a string, so you're effectively doing the work twice, creating the string and then appending it. If it's possible for you to pass the StringBuilder itself to those createTagRow calls, that would also be a lot less... scary.

Actually, on a second look-over, it seems that this code is building a SQL query. Parameterize that query NOW. No excuses. That way you won't even need to worry about string.Format vs. StringBuilder vs. concatenation, the DB library will handle it all for you.

Upvotes: 5

pmarflee
pmarflee

Reputation: 3428

Your code would probably be more readable if you used String.Format to build each line of the query, making use of its placeholder syntax to interpolate the data from your row object into the query text.

Upvotes: 0

Related Questions