Comments on Moserware: Boy Scout Check-ins

Mark Miller: it's a good point you mentioned that ...

2008-11-23T16:54:00.000-05:00

Mark Miller: it's a good point you mentioned that things evolve over time. This perhaps makes it even more important to do Boy Scout Check-ins. You might have done something in a not-as-clear way in the past to work around a limitation. Thankfully, the most obvious/clear way is typically optimized later. Within reason, optimize for clarity and then let the tools catch up :)

Elaine: It's painful; I've been there. People don't want to make even tiny improvements for fear that their name will be the last edit on a file (and thus responsible for bug fixes). This cultures seems to kill the fun from and the code quality deteriorates... forcing a mediocre code and programmers feeling like their in a tar pit.

Hopefully your new job has a better culture? :)

Good topic, Jeff.

At my last place of devel...

2008-11-23T15:34:00.000-05:00

Good topic, Jeff.

At my last place of development, we were told *NOT* to change anything outside specifically what we were told to change. Why? Because you didn't want "ownership" of the code if it didn't apply to the changes (and case number) you were making.

It got to the point where some entire systems needed to be rewritten (some code dating back to the early '80's) because no one knew what the hell was going on -- they only knew what went in and what came out.

Microsoft itself used to recommend using the Strin...

2008-11-16T01:07:00.000-05:00

Microsoft itself used to recommend using the String.Concat() technique instead of "+", because in .Net 1.x the compiler did NOT optimize concatenations. The recommendation I heard was it was fine to have 3 or 4 concatenations in a row using "+". If there was more than that, you should've used String.Concat(). Also, if you were concatenating in a loop, use String.Concat() or StringBuilder every time.

Optimization was spotty. For example, in C# 1.0 if you wrote:

for (int i = 0; i < array.Length; i++)
{...}

the compiler would optimize the the call to array.Length, only calling it on the first iteration. It did not do this in VB.Net 1.0, causing array.Length to be called each pass through the loop.

So I'd say don't beat yourself up that you used String.Concat() all the time on the projects you did a few years ago. You were doing a good thing. Now with .Net 3.x it looks like that sort of thing is no longer necessary.

As for the admonitions about refactoring, I'm all for that. When I've worked on projects I refactor bad code every chance I get. The catch is finding the chance to do it. My past experience is software shops don't want to spend time on refactoring, because it's not considered a value add. Every time I did it I had to do it on the sly. I'd have a list of bug fixes to do. I'd add in a little time, nothing significant, to fix up some messy code, if that was possible. Sometimes some messy code was depended upon by a lot of other objects, so refactoring it would take too long. I had to leave it. Now, with the refactoring tools that are available, perhaps I'd be able to do even the tough cases.

Interesting, I threw some code in a dll and put it...

2008-11-11T11:14:00.000-05:00

Interesting, I threw some code in a dll and put it in reflector and did it compile down to string.concat(object[] { ... } ) so either where i originally read that about string addition was either wrong or they optimized it at some point.

dotnetchris:

I thought the same thing until...

2008-11-10T20:00:00.000-05:00

dotnetchris:

I thought the same thing until I looked at the generated code:

Take two methods:

string GenerateH1WithConcat(string text) { return string.Concat("<h1>", text, "</h1>"); }

and

string GenerateH1WithPlusOperator(string text) { return "<h1>" + text + "</h1>"; }

You'll see that the compiled IL code is identical:

maxstack 3
.locals init ([0] string CS$1$0000)
L_0000: nop
L_0001: ldstr "<h1>"
L_0006: ldarg.0
L_0007: ldstr "</h1>"
L_000c: call string [mscorlib]System.String::Concat(string, string, string)
L_0011: stloc.0
L_0012: br.s L_0014
L_0014: ldloc.0
L_0015: ret

If you look at the results in Reflector using the C# decompiler, both will produce the same C# code (which uses +'s since it looks nicer)

In both cases, the performance tuned 3 argument string concatenation is used that more or less does this:

int length = (str0.Length + str1.Length) + str2.Length;
string dest = FastAllocateString(length);
FillStringChecked(dest, 0, str0);
FillStringChecked(dest, str0.Length, str1);
FillStringChecked(dest, str0.Length + str1.Length, str2);

which performs one allocation and a fast fill of it. No matter how many things you + together, you'll still get a relatively fast result. After 4 arguments, a generic version is used that does the same thing but in a loop, which is slightly slower.

In the case of building up a string over the course of several lines or in a loop, it makes sense to use a StringBuilder to avoid a lot of intermediate strings. This is the exception I mentioned in the post.

Am I missing something?

As for String.Empty and "", I'll leave that to a personal preference of balancing performance and readability. Whatever you do, just be consistent and I'd be happy :)

One note I would like to toss in there, it really ...

2008-11-10T15:22:00.000-05:00

One note I would like to toss in there, it really is a worthwhile change to replace string addition to use string.Concat. String addition creates a new string for every single addition instead of a single result string that string.Concat does.

String addition also can cause issues with string interning so it is a worthwhile "boy scout" change to make, it's also worth while to replace all "" instances with string.Empty.

Just my two cents.

shawn: glad to hear that a real scout liked it! :)...

2008-10-31T19:14:00.000-04:00

shawn: glad to hear that a real scout liked it! :)

anonymous: Thanks! Feel free to subscribe to my RSS feed (which includes an email option) at http://feeds.feedburner.com/Moserware

Fantastic post. I don't know if you have a newsle...

2008-10-31T14:44:00.000-04:00

Fantastic post. I don't know if you have a newsletter, sir, but I will subscribe to it!

This is great advice, and I absolutely love the Bo...

2008-10-31T13:26:00.000-04:00

This is great advice, and I absolutely love the Boy Scout analogy, since I've been actively part of scouting for most of my life.