0

The getter would get a message about the state of the struct it's in, which is defined by a combination of the struct's properties, which are finite.

First, this isn't about micro-optimization, I just think that: 

StringBuilder msg = new StringBuilder();
...
msg.AppendLine("static string");
...

looks cleaner then: 

String msg = String.Empty;
...
msg = String.Concat(msg, "static string", System.Environment.NewLine)
...

so just an aesthetic choice.

The msg variable has to be initialized empty either way, so the extra line doesn't bug me. But how bad is it to construct a new StringBuilder in a field that returns a string?

EDIT I just wanted to know if putting a StringBuilder constructor in a getter was going to open up a huge can of worms, be ridiculous overhead, some never use constructors in getters kind of anti-pattern thing, etc... not to know everybody's favorite most performant way to concat non-looping strings.

EDIT2 What is the usual, best practice, performance threshold for a getter and does a StringBuilder constructor fall below that?

capn
  • 512
  • 7
  • 14
  • Not sure I'd have done it for something this simple, but I do it quite often in more complex scenarios. – Tony Hopkinson May 14 '12 at 21:04
  • Just FYI, I know your question isn't really about structs, but the one statement: "...get a message about the state of the struct... " indicates that you may be creating a mutable struct, which is generally frowned on in C#. See: http://stackoverflow.com/questions/441309/why-are-mutable-structs-evil – Nathan May 14 '12 at 21:14
  • @Nathan Its not mutable, it gets constructed differently, with different parameters, it has a state. It will be passed around by value. I said nothing to the effect of setters ;) – capn May 14 '12 at 21:21
  • @Tony, are you saying you've put StringBuilders in getters before (just more complex getters)? – capn May 14 '12 at 21:55
  • 1
    @capn, yep. Things like returning one error / validation string for a log on a collection of somethings. – Tony Hopkinson May 15 '12 at 14:53

4 Answers4

5

Why not do this?

msg + "static string" + Environment.NewLine

It will compile to the same as your second example.

Update

You changed your code so that it appears that you want to create a very large string containing lots of lines.

Then I guess it's fine to use StringBuilder, but I'd suggest that you make it a method (how about overriding ToString?) rather than a property so that callers are less likely to assume that it's cheap to call.

Mark Byers
  • 811,555
  • 193
  • 1,581
  • 1,452
  • +1 Aesthetically this is the simpliest, readable choice. Choosing not to do `String.Concat`, but then thinking about using a `StringBuilder` is very strange to me. – Erik Philips May 14 '12 at 20:59
  • Never liked using + for concatenation myself, but that's probably a habit I developed in other languages. – Tony Hopkinson May 14 '12 at 21:04
  • Because + looks bad, you have to use an assignment operator, and AppendLine looks better, IMHO. Again, its aesthetics, not that you're wrong. – capn May 14 '12 at 21:10
  • 1
    @capn do you also write `Decimal.Add(1M, 1M)` rather than `1M + 1M` ? – Magnus May 14 '12 at 21:18
  • @Magnus I also get 2M instead of "11" when I use the operator like that. This seems to make more sense to me ;) – capn May 14 '12 at 21:25
  • @Magnus, I also like to use CONCAT in Oracle queries involving two or three strings instead of ||, even though I use it all the time for non-bitwise boolean expressions everywhere else, I'm just crazy like that. – capn May 14 '12 at 22:09
  • @Mark thanks for your edit, sorry it wasn't more clear before. Yes, I was thinking about using a method to indicate that it isn't computationally cheap, I guess what I really was wondering is: is it **that** computationally expensive or is it just bike shedding? – capn May 14 '12 at 22:46
  • @capn: If it's a property someone might write `foreach (string word in checkWords) { if (foo.YourProperty.Contains(word)) {...} }`. If it's a method they will probably write this instead: `string s = foo.YourMethod(); foreach (string word in checkWords) { if (s.Contains(word)) { ... } }`. The difference in performance could be quite large. If it's unlikely that your property will be used in a loop, it probably doesn't matter. But it's easier to make it a method than to figure out whether or not someone in the future might have problems because of your decision to make it a property. – Mark Byers May 14 '12 at 22:51
  • @Mark: Sure, worst case scenario is that its used in a loop. How bad is that worst case scenario, exactly? What is the usual, best practice, performance threshold for a getter and does a StringBuilder constructor fall below that? That is my question. Maybe I should edit that in there. – capn May 14 '12 at 23:02
1

From a performance point of view, with the data supplied (three substrings), the String.Concat is better.

But, if inside the getter, you have lines like if(state == 0) that disrupt the efficiency of Concat or + operator then use StringBuilder for its good efficiency in string memory handling and for its clear syntax on AppendLine. Look at this site for data on StringBuilder vs Concat vs + and for some info on StringBuilder tips and mistakes

Steve
  • 213,761
  • 22
  • 232
  • 286
  • this isn't about micro-optimization see link: http://www.codinghorror.com/blog/2009/01/the-sad-tragedy-of-micro-optimization-theater.html – capn May 14 '12 at 21:11
  • Sorry, but I am a little confused now, The msg var is a StringBuilder? and `msg = String.Concat(msg, "static string", System.Environment.NewLine)` will compile? – Steve May 14 '12 at 21:24
  • Sorry was using the same variable name for two different objects, a little clearer now maybe? – capn May 14 '12 at 21:30
  • Yes, thanks now it is clearer. So, in the case of StringBuilder you write the object constructor, the initialization code and then return the stringbuilder converted via ToString(), while in the answer above from Mark Bryers there is just one line. I will go for the Mark answer, but if we exclude the optimization then it's a personal choiche. (Of course I assume you don't have any more strings to append to the getter result). – Steve May 14 '12 at 21:40
  • No, there are a number of strings to append depending on the state of the struct, hence the ellipses. I have to initialize and return either way, so I'm not gaining less lines of code either way. AppendLine is very simple english to me rather than also concat'ing Envirnment.NewLine. I just want to know how much overkill a StringBuilder constructor in a getter is. – capn May 14 '12 at 21:46
0

String.Concat function outperforms the StringBuilder by 2.3 times when not using lots of strings. Also,if for example you write code like "a" + "b" + "c" + "d" + "f" the compilter will compile it to use string.Concat(string[]) in the IL code.

yonigozman
  • 707
  • 8
  • 15
0

In addition to answers present should say, that considering that you're talking about properties of the struct (I don't think thay are 1000s, but..), it's important to mantion fact, that the difference is not only in the semantics present, but in actual functinality too.

To be shorter and clearer:

+ and string.Concat on every call create a new string object.

If you use StringBuilder, instead, it operates on one single string buffer. So it's very convinient, from performance point of view, to use StringBuilder if you have a long string to compose.

Tigran
  • 61,654
  • 8
  • 86
  • 123