String foreach problem

[ApeX]

Hi guys, i hace a question

i have a datagrid

col1 col2
-----------------
D text0
text1
text2
D text3
D text4

i need to do a foreach and everywhere where's a "D" in col1 to add the
value of col2 into a string, so the final string would be "text0" +
"," + "text3" + "," + "text4";

for example:

string Uvjet()
{
foreach (DataRow red in dt.Rows)
{
if (red["col1"].ToString() == "D")
{
string uvjet = red["col2"].ToString() + ",";
}
}
return uvjet;
}


...only this fills the string "uvjet" only with one value....and i want
it to be finally filled with all necessary values, like:
"text0" + "," + "text3" + "," + "text4"

if you have any ideas, please help...
THANX!

 

[Mythran]

string Uvjet()
{
foreach (DataRow red in dt.Rows)
{
if (red["col1"].ToString() == "D")
{
string uvjet = red["col2"].ToString() + ",";
}
}
return uvjet;
}


..only this fills the string "uvjet" only with one value....and i want
it to be finally filled with all necessary values, like:
"text0" + "," + "text3" + "," + "text4"

if you have any ideas, please help...
THANX!

From what I see, that code won't even compile...

try:

string Uvjet()
{
string uvjet = string.Empty;
foreach (DataRow red in dt) {
if (red["col1"].ToString() == "D") {
if (uvjet != string.Empty) uvjet += ";";
uvjet = red["col2"].ToString();
}
}
return uvjet;
}

Note: The above is NOT functionally equivalent to what you posted earlier.
This ONLY adds a semi-colon on iterations after the first iteration (so you
won't have a single entry with a semi-colon at the end of the string).

Also note, I haven't tested it so it may not compile either

HTH,
Mythran

 

[=?ISO-8859-2?Q?G=F6ran_Andersson?=]

Hi guys, i hace a question

i have a datagrid

col1 col2
-----------------
D text0
text1
text2
D text3
D text4

i need to do a foreach and everywhere where's a "D" in col1 to add the
value of col2 into a string, so the final string would be "text0" +
"," + "text3" + "," + "text4";

for example:

string Uvjet()
{
foreach (DataRow red in dt.Rows)
{
if (red["col1"].ToString() == "D")
{
string uvjet = red["col2"].ToString() + ",";
}
}
return uvjet;
}


..only this fills the string "uvjet" only with one value....and i want
it to be finally filled with all necessary values, like:
"text0" + "," + "text3" + "," + "text4"

if you have any ideas, please help...
THANX!

As Mythran mentioned, your code will not even compile. As you have
declared the uvjet string inside the if statement, that's the scope of
the variable, and you can't use it outside of it's scope.

If you correct the code by declaring the string outside the loop, you
are still just assigning the value to the string, that means that you
will be replacing the value from any previous row. You have to
concatenate the strings.

This is however a good example where concatenating strings is a bad
idea. As strings are immutable in .NET, you will be creating a new
string for every concatenation, copying the previous string data into
the new string. That means that you will be moving more and more data
for every additional value that you add to the string. The amount of
memory used is exponetially proportional to the number of strings added.

The solution is a StringBuilder.

Whenever you are concatenating string, and you don't know beforehand
exactly how many strings there will be, it's better to use a StringBuilder.

StringBuilder builder = new StringBuilder();
foreach (DataRow red in dt.Rows) {
if (red["col1"].ToString() == "D") {
if (builder.Length > 0) {
builder.Append(',');
}
builder.Append(red["col2"].ToString());
}
}
return builder.ToString();

Note that this code does not add an extra comma at the end of the
string, unlike what your original code would have done.

 

[Peter Duniho]

Hi guys, i hace a question

i have a datagrid

Your code appears to be using a DataTable, not a DataGrid. There's a
difference.

col1 col2
-----------------
D text0
text1
text2
D text3
D text4

i need to do a foreach and everywhere where's a "D" in col1 to add the
value of col2 into a string, so the final string would be "text0" +
"," + "text3" + "," + "text4";

So, you only want commas between the fields, and not one at the end?

While Mythran's reply is wrong in at least a couple of ways, it does offer
one insight that should be useful to you: initialize a string to the empty
string, and then append text with each iteration as appropriate. He is
also correct that your code doesn't compile (since you use "uvjet" outside
of the code block in which it's declared).

So:

string Uvjet()
{
string uvjet = "";
foreach (DataRow red in dt.Rows)
{
if (red["col1"].ToString() == "D")
{
if (uvjet != "")
{
uvjet += ",";
}
uvjet += red["col2"].ToString();
}
}
return uvjet;
}

If you like, you could use a StringBuilder instance with the Append()
method instead of using the + concatenation operator with the String class.

Pete

 

[Peter Duniho]

From what I see, that code won't even compile...

Just FYI...

I think your code will compile, but it has some problems:

try:

string Uvjet()
{
string uvjet = string.Empty;
foreach (DataRow red in dt) {
if (red["col1"].ToString() == "D") {
if (uvjet != string.Empty) uvjet += ";";

He wanted commas, not semicolons.

uvjet = red["col2"].ToString();

This line simply replaces "uvjet" with a new string instance. No
concatenation.

}
}
return uvjet;
}

Pete

 

[Mark Wilden]

A StringBuilder is used to build a string. I can't imagine needing a string
so long that using a StringBuilder instead of creating new strings would be
significant.

 

[Peter Duniho]

A StringBuilder is used to build a string.

Which is, in fact, what the OP is trying to do.

I can't imagine needing a string
so long that using a StringBuilder instead of creating new strings would
be
significant.

Ever? Then why does the StringBuilder class exist?

Seriously though: you and I have no idea what the length of the string
is. It could be hundreds of kilobytes or larger for all we know.

It may well be that the OP is dealing with just a few rows of data and
thus concatenation is fine. But it is just as true that for all we know
he's got thousands of rows of data, and that using a StringBuilder would
in fact be noticeably better.

Pete

 

[=?ISO-8859-1?Q?G=F6ran_Andersson?=]

A StringBuilder is used to build a string. I can't imagine needing a string
so long that using a StringBuilder instead of creating new strings would be
significant.

How about this for a real world example:

http://www.codeproject.com/script/comments/forums.asp?forumid=1649&select=1519247&df=100&tid=1519247

 

[Jon Skeet [C# MVP]]

A StringBuilder is used to build a string. I can't imagine needing a string
so long that using a StringBuilder instead of creating new strings would be
significant.

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

The length of the string that isn't the only significant factor - it's
the amount of redundant data which needs copying. Building up a 50K
string one character at a time (using string concatenation) is painful
- but concatenating two 500K strings is quick.

Jon

 

[Mythran]

From what I see, that code won't even compile...

Just FYI...

I think your code will compile, but it has some problems:

try:

string Uvjet()
{
string uvjet = string.Empty;
foreach (DataRow red in dt) {
if (red["col1"].ToString() == "D") {
if (uvjet != string.Empty) uvjet += ";";

He wanted commas, not semicolons.

uvjet = red["col2"].ToString();

This line simply replaces "uvjet" with a new string instance. No
concatenation.

whoops, I originally had += there Oh well....that's why the disclaimer
was in there...

Kip

 

[Mark Wilden]

Seriously though: you and I have no idea what the length of the string
is. It could be hundreds of kilobytes or larger for all we know.
It may well be that the OP is dealing with just a few rows of data and
thus concatenation is fine. But it is just as true that for all we know
he's got thousands of rows of data, and that using a StringBuilder would
in fact be noticeably better.

The poster is using a DataGrid. It's not good practice to put a large number
of items in a DataGrid. What would the user do with thousands of rows?.

 

[Mark Wilden]

How about this for a real world example:

http://www.codeproject.com/script/comments/forums.asp?forumid=1649&select=1519247&df=100&tid=1519247

OK, when you're creating a string from all the bytes of a 120K file, I can
see using StringBuilder.

///ark

 

[Mythran]

The poster is using a DataGrid. It's not good practice to put a large
number of items in a DataGrid. What would the user do with thousands of
rows?.

The poster stated he/she/it is using a DataGrid, but the code shows a
DataTable with DataRow's, so we are assuming he/she/it meant DataSet.



Mythran

 

[Mark Wilden]

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

See it? I've practically memorized it!

The length of the string that isn't the only significant factor - it's
the amount of redundant data which needs copying. Building up a 50K
string one character at a time (using string concatenation) is painful
- but concatenating two 500K strings is quick.

My point was that I couldn't imagine building up a 50K string one character
at a time. Goran posted an example of doing something like that, but I'd
still say that was very rare.

At least your article has disabused people of the notion that concatentation
should always be done using StringBuilder. However, a lot of folks still
think that StringBuilder should be used whenever concatenating in a loop,
and I think this is premature optimization most of the time. In general, I'd
call long strings (thousands of characters) a code smell.

For example, Goran's example was about "showing users a string
representation of a 120Kb file in Hex-format within a Windows forms
textbox." How useful would that actually be to anyone? I'm not saying it's
impossible; just that I'm doubtful.

///ark

 

[Mark Wilden]

The poster stated he/she/it is using a DataGrid, but the code shows a
DataTable with DataRow's, so we are assuming he/she/it meant DataSet.

That makes it more likely that a long string would be generated from the
rows, but I'd still like to know what is planned to be done with that
string.

///ark

 

[Jon Skeet [C# MVP]]

My point was that I couldn't imagine building up a 50K string one character
at a time. Goran posted an example of doing something like that, but I'd
still say that was very rare.

You wouldn't normally do exactly that, but I've seen plenty of real
world examples where people *have* built up a very large string line by
line. For instance, here's a really bad way of reading a text file:

string text = "";

using (StreamReader reader = ...)
{
string line;
while ((line = reader.ReadLine()) != null)
{
text = text + line;
text = text + "\r\n";
}
}

I've seen code like this posted, with people wondering why it's taking
so long to read a 10MB file. This isn't speculation, it's experience.
Changing the code to use a StringBuilder makes the code much, much
faster. (Yeah, using ReadToEnd also works

At least your article has disabused people of the notion that concatentation
should always be done using StringBuilder. However, a lot of folks still
think that StringBuilder should be used whenever concatenating in a loop,
and I think this is premature optimization most of the time. In general, I'd
call long strings (thousands of characters) a code smell.

I use a StringBuilder when I don't have concrete guarantees about the
number of iterations. Aside from anything else, it actually documents
what's going on more clearly, IMO: "For this section of code we're
building up the string... and now we've finished."

For example, Goran's example was about "showing users a string
representation of a 120Kb file in Hex-format within a Windows forms
textbox." How useful would that actually be to anyone? I'm not saying it's
impossible; just that I'm doubtful.

Long strings can be useful quite often. Not all the time, but quite
often. Why introduce an O(n^2) way of building them up when an O(n) way
exists which is very straightforward to use?

 

[Mark Wilden]

You wouldn't normally do exactly that, but I've seen plenty of real
world examples where people *have* built up a very large string line by
line. For instance, here's a really bad way of reading a text file:

This is really my point. The advice against using + for concatenating
strings is usually accompanied by a very bad reason for concatenating in the
first place.

I use a StringBuilder when I don't have concrete guarantees about the
number of iterations.

I don't require a concrete guarantee - just some idea of the number of
iterations. If there are a lot of iterations, StringBuilder is the best
choice - agreed. I'm just questioning how many times that happens. My
definition of "a lot" is the same as yours, I suspect - where performance
degradation is noticeable.

Long strings can be useful quite often. Not all the time, but quite often.

"Quite often" is obviously subjective. All I can say from my experience is
that long strings are rarely useful (as in, I might need to use them once a
year). My suspicion (and it's only that) is that many (there's that word
again) uses of long strings are incorrect (which goes for the examples I've
seen so far here).

Why introduce an O(n^2) way of building them up when an O(n) way
exists which is very straightforward to use?

Because + is easier to read than a method call. In the example that I saw
most recently, there were five lines of code just to put a string together -
because the programmer was afraid of +.

///ark

 

[Jon Skeet [C# MVP]]

This is really my point. The advice against using + for concatenating
strings is usually accompanied by a very bad reason for concatenating in the
first place.

I didn't say that the reason for reading the text file line by line was
bad. For instance, there might have been a good reason to have the
whole thing in memory at a time as a string, having prepended the line
number to each line.

The only really bad thing about the example was using concatenation.

I don't require a concrete guarantee - just some idea of the number of
iterations. If there are a lot of iterations, StringBuilder is the best
choice - agreed. I'm just questioning how many times that happens. My
definition of "a lot" is the same as yours, I suspect - where performance
degradation is noticeable.

All I can say is that my experience on the groups is that it's far from
unheard of for the degredation to be noticeable.

"Quite often" is obviously subjective. All I can say from my experience is
that long strings are rarely useful (as in, I might need to use them once a
year). My suspicion (and it's only that) is that many (there's that word
again) uses of long strings are incorrect (which goes for the examples I've
seen so far here).

"Incorrect" sounds very concrete to me. If there's more than one way of
achieving an overall goal, what makes one way "incorrect"?

Because + is easier to read than a method call. In the example that I saw
most recently, there were five lines of code just to put a string together -
because the programmer was afraid of +.

It's certainly abused - but so is string concatenation. In both cases
it's usually because of ignorance.

 

[Mark Wilden]

I didn't say that the reason for reading the text file line by line was
bad. For instance, there might have been a good reason to have the
whole thing in memory at a time as a string, having prepended the line
number to each line.

Oh, OK. You mentioned ReadToEnd, which I'd say was a better way than
concatenation, by whatever means.

All I can say is that my experience on the groups is that it's far from
unheard of for the degredation to be noticeable.

I agree. What I'm trying to say is that sometimes we need to take a step
back and ask whether concatenating large numbers of strings is the real
problem - not the method of accomplishing it.

"Incorrect" sounds very concrete to me. If there's more than one way of
achieving an overall goal, what makes one way "incorrect"?

Again, I'm saying that it appears to me that the goal is what's incorrect.
For example, you should use Append to turn a binary file into hex digits
instead of +. But should you display a binary file in a textbox in the first
place? That's why I call long strings a "smell." Not prima facie incorrect,
but often incorrect.

///ark

 

[Peter Duniho]

The poster is using a DataGrid. It's not good practice to put a large
number
of items in a DataGrid. What would the user do with thousands of rows?.

Well, as has been pointed out, it seems safer to assume that the OP
misstated, and they are in fact using a DataSet, not DataGrid. But
whether he is or not, my point is that your statement didn't restrict
itself to a DataGrid, and as a more general statement I found it lacking.

You didn't write "I can't imagine needing a string created from a DataGrid
that is so long that using a StringBuilder instead of creating new strings
would be significant".

You did write "I can't imagine needing a string so long that using a
StringBuilder instead of creating new strings would be significant".

There's a world of difference between the two statements, and my point is
that there certainly are scenarios in which a StringBuilder makes a LOT of
sense for concatenating strings.

Pete