How would you code it?
-
PIEBALDconsult wrote:
The first is more readable, less error-prone
My example demonstrates how it's not less error prone. It's easy to mess up the numbering, or to pass in more or less parameters than expected, or to pass them in the wrong order.
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
I don't buy it. I just don't see how any variant of
Dim sqlUpdate As String = "UPDATE ls_orderitems SET OrderItemRetailPrice = '" & _
Trim(PartRetailPrice) & "', OrderItemSalePrice = '" & Trim(PartSalePrice) & _
"' WHERE OrderItemPartNum = '" & Trim(PartNum) & "', OrderItemSource = '" & _
Trim(PartSource) & "', OrderType = 'Ron Ayers MotorSports'"can possibly be considered "easier to debug".
A guide to posting questions on CodeProject[^]
Dave Kreskowiak Microsoft MVP Visual Developer - Visual Basic
2006, 2007 -
Here's a simple code snippet (in C#):
string hello = "Hello";
string cp = "CP";
DateTime today = DateTime.Today;// Desired result: "Hello CP! Today is Friday";
string option1 = string.Format("{0} {1}! Today is {3:dddd}", hello, cp, today);
string option2 = hello + " " + cp + "! Today is " + today.ToString("dddd");
Vote 1 if you prefer option1. Vote 5 if you prefer option2. I prefer option2 since it's 1. More readable 2. Less error-prone (note the subtle error in option1 which the compiler won't catch) 3. More efficient (no CPU cycles spent scanning the format string looking for matching curly braces). Cheers!
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
-
I don't buy it. I just don't see how any variant of
Dim sqlUpdate As String = "UPDATE ls_orderitems SET OrderItemRetailPrice = '" & _
Trim(PartRetailPrice) & "', OrderItemSalePrice = '" & Trim(PartSalePrice) & _
"' WHERE OrderItemPartNum = '" & Trim(PartNum) & "', OrderItemSource = '" & _
Trim(PartSource) & "', OrderType = 'Ron Ayers MotorSports'"can possibly be considered "easier to debug".
A guide to posting questions on CodeProject[^]
Dave Kreskowiak Microsoft MVP Visual Developer - Visual Basic
2006, 2007Oh, well for that you need parameters anyway.
-
What happens when it becomes
hello + " " + value + " " + value2 + value + " " + item.Format("{0:D}") + hello;? Is that still easy to visualize and have you looked at the IL that this produces?Deja View - the feeling that you've seen this post before.
And don't forget ...
value==null?"null":value.ToString()... -
What happens when it becomes
hello + " " + value + " " + value2 + value + " " + item.Format("{0:D}") + hello;? Is that still easy to visualize and have you looked at the IL that this produces?Deja View - the feeling that you've seen this post before.
Pete O`Hanlon wrote:
What happens when it becomes hello + " " + value + " " + value2 + value + " " + item.Format("{0:D}") + hello;?
I prefer it to
string.Format("{0} {1} {2}{1} {3:D}{0}", hello, value, value2, item);Although it does appear neater, I can't visualize anything here. All I see is 6 placeholders and 4 variables. Bringing it all together mentally is, to me, more difficult.Pete O`Hanlon wrote:
you looked at the IL that this produces
That's the best part. It's the equivalent of string.Concat(hello, " ", value, " ", value2, value, " ", ...);
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
-
Al Beback wrote:
I have to mentally keep substituting the placeholders for their corresponding values.
You have to do that both ways. The difference is, #1 just gives you a tiny little "this is a placeholder" mark, while #2 uses the full variable or expression (which is unlikely to be any closer in appearance to the actual result than #1's curly-brace marks, but takes up a whole lot more room. Not to mention requiring parens to separate non-string ops from string ops). Of course, it's a matter of preference - but i'm pretty set in my ways here; i use similar techniques for C++ and even JS, writing my own formatters when there's nothing suitable built-in. You have to do that both ways. The difference is, #1 just gives you a tiny little "this is a placeholder" mark, while #2 uses the full variable or expression**†**. Of course, it's a matter of preference - but i'm pretty set in my ways here; i use similar techniques for C++ and even JS, writing my own formatters when there's nothing suitable built-in. BTW - which of the above paragraphs do you prefer? ;P **†**which is unlikely to be any closer in appearance to the actual result than #1's curly-brace marks, but takes up a whole lot more room. Not to mention requiring parens to separate non-string ops from string ops.
every night, i kneel at the foot of my bed and thank the Great Overseeing Politicians for protecting my freedoms by reducing their number, as if they were deer in a state park. -- Chris Losinger, Online Poker Players?
Shog9 wrote:
which of the above paragraphs do you prefer?
1! :-D
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
-
Well, if/when you use a Console.WriteLine(), would you use the format string and parameters?
PIEBALDconsult wrote:
Well, if/when you use a Console.WriteLine(), would you use the format string and parameters?
No. I would still use concatenation for the same reasons (readability, stability, and efficiency).
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
-
leppie wrote:
WWCND?
:) I had to look that up.
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
-
-
What happens when it becomes
hello + " " + value + " " + value2 + value + " " + item.Format("{0:D}") + hello;? Is that still easy to visualize and have you looked at the IL that this produces?Deja View - the feeling that you've seen this post before.
Or if you need to include a value multiple times? One that is costly to retrieve? With Option2, at best you declare a temporary value for it; but using Option1 doesn't require that. Or if you want to include a value in more than one format, like printing an int in both decimal and hexadecimal.
-
Pete O`Hanlon wrote:
What happens when it becomes hello + " " + value + " " + value2 + value + " " + item.Format("{0:D}") + hello;?
I prefer it to
string.Format("{0} {1} {2}{1} {3:D}{0}", hello, value, value2, item);Although it does appear neater, I can't visualize anything here. All I see is 6 placeholders and 4 variables. Bringing it all together mentally is, to me, more difficult.Pete O`Hanlon wrote:
you looked at the IL that this produces
That's the best part. It's the equivalent of string.Concat(hello, " ", value, " ", value2, value, " ", ...);
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
Only for small numbers of strings. If I remember correctly it is <= 3. Anyway, performance shouldn't be much of a concern with either method.
This blanket smells like ham
-
I don't buy it. I just don't see how any variant of
Dim sqlUpdate As String = "UPDATE ls_orderitems SET OrderItemRetailPrice = '" & _
Trim(PartRetailPrice) & "', OrderItemSalePrice = '" & Trim(PartSalePrice) & _
"' WHERE OrderItemPartNum = '" & Trim(PartNum) & "', OrderItemSource = '" & _
Trim(PartSource) & "', OrderType = 'Ron Ayers MotorSports'"can possibly be considered "easier to debug".
A guide to posting questions on CodeProject[^]
Dave Kreskowiak Microsoft MVP Visual Developer - Visual Basic
2006, 2007 -
I don't buy it. I just don't see how any variant of
Dim sqlUpdate As String = "UPDATE ls_orderitems SET OrderItemRetailPrice = '" & _
Trim(PartRetailPrice) & "', OrderItemSalePrice = '" & Trim(PartSalePrice) & _
"' WHERE OrderItemPartNum = '" & Trim(PartNum) & "', OrderItemSource = '" & _
Trim(PartSource) & "', OrderType = 'Ron Ayers MotorSports'"can possibly be considered "easier to debug".
A guide to posting questions on CodeProject[^]
Dave Kreskowiak Microsoft MVP Visual Developer - Visual Basic
2006, 2007Dave Kreskowiak wrote:
can possibly be considered "easier to debug".
I never mentioned debugging. When you debug this, you step over that statement and look at the resulting string. It's the same when using Format.
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
-
And don't forget ...
value==null?"null":value.ToString()...PIEBALDconsult wrote:
And don't forget ...value==null?"null":value.ToString()...
How do you handle that with format specifiers?
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
-
Shog9 wrote:
which of the above paragraphs do you prefer?
1! :-D
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
Surprise surprise! ;P
every night, i kneel at the foot of my bed and thank the Great Overseeing Politicians for protecting my freedoms by reducing their number, as if they were deer in a state park. -- Chris Losinger, Online Poker Players?
-
malharone wrote:
Performance: string concat. is heavy, which does not happen in the string.format
That's what I thought as well. Although the OP stated that the concat. approach was more efficient, but I'm not so sure I buy his reasoning for it. I think he's fishing for "5" votes. ;) BDF
string.Concat is the fastest way to concatenate strings. StringBuilder is not faster than string.Concat; only multiple Append calls to the same StringBuilder are more efficient than multiple calls to Concat for building a single string.
-
Here's a simple code snippet (in C#):
string hello = "Hello";
string cp = "CP";
DateTime today = DateTime.Today;// Desired result: "Hello CP! Today is Friday";
string option1 = string.Format("{0} {1}! Today is {3:dddd}", hello, cp, today);
string option2 = hello + " " + cp + "! Today is " + today.ToString("dddd");
Vote 1 if you prefer option1. Vote 5 if you prefer option2. I prefer option2 since it's 1. More readable 2. Less error-prone (note the subtle error in option1 which the compiler won't catch) 3. More efficient (no CPU cycles spent scanning the format string looking for matching curly braces). Cheers!
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
Al Beback wrote:
1. More readable
Entirely and completely subjective.
Al Beback wrote:
2. Less error-prone (note the subtle error in option1 which the compiler won't catch)
Perhaps. But you do have unit tests and system tests correct? If so then why are they not testing that code?
Al Beback wrote:
3. More efficient (no CPU cycles spent scanning the format string looking for matching curly braces).
If that is an actual performance bottleneck in your system then you have a very unusual system. Nothing in the above suggests an actual valid reason to choose either. As such it is a matter of personal choice. One real reason for choosing the Format version is because it support internationalization where the other does not. That of course is dependent on whether the application does have an international market and whether the multitude of other areas required for internationalization are actively being worked. And I can only note that actually moving this into a resource to support internationalization has many other possibilities for errors so one really better be testing everything.
-
string.Concat is the fastest way to concatenate strings. StringBuilder is not faster than string.Concat; only multiple Append calls to the same StringBuilder are more efficient than multiple calls to Concat for building a single string.
Thanks. That makes sense. Do you think it's even an issue in the very simple cases shown in the start of this thread? That's creating a string with a few concatenations versus String.Format. BDF
-
PIEBALDconsult wrote:
Well, if/when you use a Console.WriteLine(), would you use the format string and parameters?
No. I would still use concatenation for the same reasons (readability, stability, and efficiency).
Man is a marvelous curiosity ... he thinks he is the Creator's pet ... he even believes the Creator loves him; has a passion for him; sits up nights to admire him; yes and watch over him and keep him out of trouble. He prays to him and thinks He listens. Isn't it a quaint idea. - Mark Twain
Well consistency is good too.
-
Only for small numbers of strings. If I remember correctly it is <= 3. Anyway, performance shouldn't be much of a concern with either method.
This blanket smells like ham
It's <= 4. For more strings, it's still a single call to Concat(string[]), which means a temporary string array is created. This is still better than creating a temporary StringBuilder object, which creates multiple String objects when it needs to make the string larger. I only use StringBuilder when it is not possible to use a single call to string.Concat - e.g. in a loop like this: StringBuilder b = new StringBuilder(); foreach (SomeClass c in list) { b.Append(c.Text); b.Append(','); } return b.ToString(); The algorithm is O(n), with multiple string.Concat calls it would be O(n^2). Somehow the advice that StringBuilder should be used in those cases became the urban legend "StringBuilder is always faster in a loop". The advice only applies if the loop is appending to a single string; in cases where one would need multiple StringBuilder objects (or where one would re-use a StringBuilder object after calling ToString() on it), string.Concat is faster: string[] ConcatPairs(string[] a, string[] b) { string[] c = new string[a.Length]; for (int i = 0; i < a.Length; i++) { c[i] = a[i] + b[i]; } return c; } And String.Format is a real performance killer: not only is it required to parse the format string, it also puts all arguments in a temporary object[] array (value types are boxed if required), tries to cast all arguments to IFormattable, then calls ToString() on them (a virtual method call, or interface method call if the object implements IFormattable). And in the end it uses StringBuilder for the concatenation (since string.Format contains a loop appending to the same string), which even alone is slower than a single string.Concat call.