Foreach copy and text

Question

My code:

List<string> fileList = Directory.GetFiles(path).ToList();
foreach (string file in fileList)
{
    File.Copy(file, biosAllDir + @"\" + Path.GetFileName(file), true);
    string fileWithoutPath = Path.GetFileName(file);
    outputTextbox.Text = $"Copied: {fileWithoutPath}" + Environment.Newline; 
}

This foreach loop copies all the files in the list fine but when it comes to adding text to the outputTextbox it just adds one line (the last one) and not any other, do you know why?

Thanks

You're replacing the content of the textbox each time, not appending to it (i.e. `=` instead of `+=`) — Diado, Jul 24 '19 at 11:26

SᴇM · Accepted Answer · 2019-07-24T11:32:32.390

2

On every iteration, you are just assigning the result to your textbox. To add them up - you need to use += instead of =

outputTextbox.Text += $"Copied: {fileWithoutPath}" + Environment.Newline;

But will be better to use StringBuilder for this case.

StringBuilder sb = new StringBuilder();
List<string> fileList = Directory.GetFiles(path).ToList();
foreach (string file in fileList)
{
    File.Copy(file, biosAllDir + @"\" + Path.GetFileName(file), true);
    string fileWithoutPath = Path.GetFileName(file);
    sb.AppendLine($"Copied: {fileWithoutPath}");
}

outputTextbox.Text = sb.ToString();

References: StringBuilder, When to use StringBuilder

edited Jul 24 '19 at 11:32

answered Jul 24 '19 at 11:27

SᴇM

7,024
3
24
41

Why is it better in this case to use `StringBuilder`?. Feels to me (admittedly a feeling, haven't looked at the sourcecode for `StringBuilder`) this is one of the cases where it either doesn't matter or the `StringBuilder` will be slower due to added management around it. So I' interested in what it does that makes it better. – Jul 24 '19 at 11:37
@Knoop at the bottom, I've added [When to use StringBuilder](https://stackoverflow.com/questions/1825781/when-to-use-stringbuilder) link. – SᴇM Jul 24 '19 at 11:38
https://stackoverflow.com/a/530014/10608418. This is where I ended up, states specifically that if you're using the intermediate result (like this case) `StringBuilder` is not going to help you... – Jul 24 '19 at 12:20
@Knoop This one is this case: _"Definitely use StringBuilder when you're concatenating in a non-trivial loop - especially if you don't know for sure (at compile time) how many iterations you'll make through the loop. For example, reading a file a character at a time, building up a string as you go using the += operator is potentially performance suicide."_ – SᴇM Jul 24 '19 at 12:21
Thanks for the response. I guess it's way too hot here, for some reason I thought the `outputTextbox.Text = sb.ToString()` was inside the loop. You're absolutely right this is a case where `StringBuilder` shines, my aplogies. – Jul 24 '19 at 12:26
@Knoop It's ok, have a nice day) – SᴇM Jul 24 '19 at 12:28

Prasad Telkikar · Answer 2 · 2019-07-24T11:45:08.693

In your case you are just assigning $"Copied: {fileWithoutPath}" + Environment.Newline; to outputTextbox.Text variable in each foreach loop iteration. Instead of assignment, you need to append that string to previously stored value. You can do it using StringBuilder

You can use StringBuilder to store all file paths and assign string from string builder out of foreach loop to outputTextbox.Text

You can try below code

List<string> fileList = Directory.GetFiles(path).ToList();
StringBuilder sb = new StringBuilder();

foreach (string file in fileList)
{
    string fileWithoutPath = Path.GetFileName(file); // Refactored to remove duplicate code.
    File.Copy(file, biosAllDir + @"\" + fileWithoutPath, true);
    sb.AppendLine($"Copied: {fileWithoutPath}");
}

outputTextbox.Text = sb.ToString();

Foreach copy and text

2 Answers2