Pregunta

I am reviewing the following code.

private string MakeKey(int SnapshotID, string UserName, string BrandName)
{
    return string.Format("{0}:{1}:{2}", SnapshotID.ToString(), UserName, BrandName);
}

The SnapshotID.ToString() redundantly calls the ToString() Method.
What are the potential issues that could occur? e.g. speed.

EDIT
I have added a test if anyone wants to refine: http://pastebin.com/gynNjwT1

With ToString:      0.010884
Without ToString:   0.001446

With ToString:      0.005506
Without ToString:   0.002852

With ToString:      0.001155
Without ToString:   0.009117

With ToString:      0.003210
Without ToString:   0.001546
¿Fue útil?

Solución

In this code...no, it's just redundant and even slightly slower because String.Format() will call again ToString() on the string itself.

In case String.Format() is used with an IFormatProvider an explicit ToString() will override it.

Edits (from comments): note about performance. It may be or not slower (an useless ToString() call) compared to primitive type (int) boxing. We should measure this but it's pretty hard to say. What's slower? A virtual function call or a boxing? Does it really matter when compared to String.Format() total time? It has been already answered here on SO. IMO if it has been done as a smart optimization it's pretty useless.

Performance

OP made a small test for this (see comments), just for fun, results on my test machine:

Test               Time [ms]
With ToString      0.002929
Without ToString:  0.003414

I rewrote test to do not use threads (because it'll add more variables, cores may differ and OS will load them dynamically). With this (over simple!) test code (derived from OP test):

Stopwatch sw = new Stopwatch();
sw.Start();
for (int x = 0; x < Count; x++)
{
    _temp = MakeKeyToString(1, "Ashley", "MyBrand");
}
sw.Stop();
TimeSpan test = TimeSpan.FromMilliseconds((double)sw.ElapsedMilliseconds);

I get that ToString() version is faster (for Count = 1000000) in average around 10/20 nanoseconds. IMO to measure something so small we need a much better test environment and a more professional approach. Changing his code for String.Format() to use IFormatProvider:

string MakeKeyToString(int SnapshotID, string UserName, string BrandName)
{
    return string.Format(CultureInfo.InvariantCulture,
        "{0}:{1}:{2}", SnapshotID.ToString(), UserName, BrandName);
}

Change everything again: without ToString() is faster for 200 nanoseconds. Again too small to measure in this way.

Conclusions

You can't even start to consider this an optimization, too many factors will play around that and it's so small that will go unnoticed compared to total String.Format() time. What's worse it may introduce subtle bugs if you'll change String.Format() to use an IFormatProviderbecause it'll make you stop and think "Why this? There should be a culture-related reason. Maybe..." when in reality...(probably) there is not.

Otros consejos

In this example, by calling Int32.ToString() you are avoiding the overhead of boxing your int into an Object.

However the overhead of calling .ToString(), then having String.Format call .ToString() on that may outweight the overhead of the boxing. And all of it is negligible in respect to String.Format.

Nope, The code is redundant and stop. Also SnapshotID cannot be null since it is an integer and not a nullable integer (int?). In this case the ToString method will be called twice I think (one explicitly and one on the result of the first call) so maybe some performance differences can be faced.

Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top