Log strings...

  • Thread starter Thread starter Nick Z.
  • Start date Start date
N

Nick Z.

I do some logging like so:

Log.Error("The song was not found. Artist - " + artist + ", Title - " +
title + "\n" + exception.ToString());

Now I know that building strings in that way is innefficient, but using
the String builder seems too complicated for the taks.

StringBuilder b = new StringBuilder();
b.Append("The song was not found. Artist - ");
b.Append(artist);
b.Append(", Title - ");
b.Append(title);
b.Append("\n");
b.Append(exception.ToString());

Log.Error(b.ToString());

Yea this should be more efficient, but looking at this mess there just
*has* to be a better way, so what is it?

Thanks,
Nick Z.
 
Nick,

Actually, this will be pretty quick. This compiles down to a call to
Concat. Concat will cycle through the strings, calculating the final length
of the string. Then, it will copy the strings over to the new string. It
might actually beat out StringBuilder in this case (since StringBuilder
would overallocate). Not by much (probably).

For single concatenations, Concat (or +) is the way to go. For things
in a loop where you will be appending to the previous result, StringBuilder
is better.

Hope this helps.
 
Ah, now it all makes sense, thanks.
I knew the compiler has to be smarter than doing this one string at a
time, but everybody keeps saying, use StringBuilder, thats where the
confusion started.

Thanks again.
Nick Z.
 
Nick,

Yes, there typically tends to be confusion in this area.

Basically, for one shot operations, where the outcome is not a factor in
the next concatenation operation, the Concat/+ route is the best way to go.
For example:

// Add N strings together
public void Write(string str1, string str2, string str3)
{
Console.Writeline(str1 + str2 + str3);
}

When you are in a tight loop, and you are using the result of the
concatenation for the next operation (say you are concatenating values from
a dataset), then the string builder is better. For example:

// Get a comma delimited list of the "value" column in the dataset.
public string CommaDelimitedString(DataTable table)
{
// The result.
StringBuilder result = new StringBuilder();

// Enumerate through the table.
foreach (DataRow row in table)
{
// Append the value, and a comma.
result.AppendFormat("{0},", row["value"]);
}

// Remove the last comma, if there are items in the builder.
if (result.Length > 0)
{
// Remove the last comma.
result.Remove(result.Length - 1, 1);
}

// Return the result.
return result.ToString();
}
 
Nicholas Paldino said:
Nick,

Yes, there typically tends to be confusion in this area.

Basically, for one shot operations, where the outcome is not a factor
in the next concatenation operation, the Concat/+ route is the best way to
go. For example:

// Add N strings together
public void Write(string str1, string str2, string str3)
{
Console.Writeline(str1 + str2 + str3);
}

When you are in a tight loop, and you are using the result of the
concatenation for the next operation (say you are concatenating values
from a dataset), then the string builder is better.
Click to expand...

Another way to say this is, when the compiler can optimize the String
concatenation, it will.
 
Mike,

Well, not completely. The key here is knowing what the compiler will
do. If you simply use concatenation in the case where you are concatenating
strings for the comma delimited string, you will end up doing way more than
you should.
 
not too sure about efficiency but for simplifying usage you might try
String.Format()

e.g.

String artist = "Matchbox 20";
String title = "busted";
String exceptionMessage = "Arfle, barfle, gloop";

const String FMT_MSG = "The Song was not found. Artist:
{0}, Title: {1}.\nException: {2}";
MessageBox.Show(String.Format(FMT_MSG,
artist,
title,
exceptionMessage));

hth,
Alan.
 
Nick Z. said:
I do some logging like so:

Log.Error("The song was not found. Artist - " + artist + ", Title - " +
title + "\n" + exception.ToString());

Now I know that building strings in that way is innefficient, but using
the String builder seems too complicated for the taks.

StringBuilder b = new StringBuilder();
b.Append("The song was not found. Artist - ");
b.Append(artist);
b.Append(", Title - ");
b.Append(title);
b.Append("\n");
b.Append(exception.ToString());

Log.Error(b.ToString());

Yea this should be more efficient, but looking at this mess there just
*has* to be a better way, so what is it?
Click to expand...

See http://www.pobox.com/~skeet/csharp/stringbuilder.html

Your first way of working is actually more efficient than the second.
 
Nick Z. said:
Ah, now it all makes sense, thanks.
I knew the compiler has to be smarter than doing this one string at a
time, but everybody keeps saying, use StringBuilder, thats where the
confusion started.
Click to expand...

Yes, it's one of those generalisations (like "exceptions are
expensive") which is repeated without any real consideration :(
 
message
Because that's in a loop, and the compiler can't see all the strings at
once, therefore can't optimize things.
Mike,

Well, not completely. The key here is knowing what the compiler will
do. If you simply use concatenation in the case where you are
concatenating strings for the comma delimited string, you will end up
doing way more than you should.
Click to expand...
 
Back
Top