Is this code Horror ?

Mohammad Dayyan

Hi, I've written the following code for batch renaming. Is it Horror ;) ? if so, how should I fix it?

private void rename_Click(object sender, EventArgs e)
{
DialogResult result = MessageBox.Show("Are you sure ?", "Warning", MessageBoxButtons.YesNo, MessageBoxIcon.Warning);
if (result == DialogResult.Yes)
{
Thread tr = new Thread(new ThreadStart(chnTarget));
tr.Start();
}
}

private void chnTarget()
{
this.Invoke(new ThreadStart(ChangeName));
}

bool cancel = false;
private void ChangeName()
{
Random rand = new Random(DateTime.Now.Second);
string folder_path = SelectedPathText.Text;
long progress_value = 0;
string new_filename = "";
cancel = false;

progressBar1.Minimum = 0;
progressBar1.Value = 0;

try
{
    string\[\] FullPathFileNames = System.IO.Directory.GetFiles(folder\_path);
    progressBar1.Maximum = FullPathFileNames.Length;

    if (FullPathFileNames.Length == 0) throw new Exception("No File Found \\n");

    foreach (string filename in FullPathFileNames)
    {
        if (cancel)
        {
            labelResult.Text = "Canceled";
            break;
        }

        progress\_value++;

        try
        {
            if (!radioRandom.Checked)
            {
                new\_filename = folder\_path + "\\\\" + textBoxTemplate.Text + progress\_value + Path.GetExtension(filename);
            }
            else
            {
                if (checkBoxOmitExtension.Checked)
                    new\_filename = folder\_path + "\\\\" + rand.Next();
                else
                    new\_filename = folder\_path + "\\\\" + rand.Next() + Path.GetExtension(filename);
            }

            File.Move(filename, new\_filename);
            Application.DoEvents();
        }
        catch { }

        progressBar1.Value = (int)progress\_value;
        progressBar1.Update();
    }

    labelResult.Text = "Complete";
    cancel = false;
}
catch (Exception ex)
{
    labelResult.Text = ex.Message;
}

}

The Project of above snippet code: Batch Renamer in C#[^]

Luc Pattyn

yes. Seishin# is correct all the way. :)

Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]

Prolific encyclopedia fixture proof-reader browser patron addict?
We all depend on the beast below.

Mohammad Dayyan

Well, How should I fix it? Where is the problem ?

Luc Pattyn

you really want me to repeat what he said? 1. don't use lots of File.Move() without checking first that no conflicts (file overwrites) will result. Solution: add code to get a two-pass approach, first mimic the renames, and if all seem to be successful, then actually do them. 2+3. don't create a thread that doesn't do a thing. this.Invoke(X) causes X to run on the main thread. And don't abuse Application.DoEvents() Solution: choose a different threading approach, e.g. use a BackgroundWorker, put the rename operation in its DoWork, and report progress using the ProgressChanged event. :|

Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]

Prolific encyclopedia fixture proof-reader browser patron addict?
We all depend on the beast below.

Mohammad Dayyan

Thanks a lot.

Luc Pattyn

you're welcome. :)

Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]

Prolific encyclopedia fixture proof-reader browser patron addict?
We all depend on the beast below.

tangname

well done! :)