Displaying a new image document in Picturebox

Question

I'm using WinForms and have a picturebox in my form. This program looks in "C:\image\" directory to find a specified image document, in my case a tif file. There is always only one picture in "C:\image\" directory.

After it locates the file, the program displays the image document in the picturebox.

When I was running this, I saw that the CPU usage was high. My goal is to make performance better or find out if there is a better way of coding this.

I have to manually go into my C:\image\ directory and delete the current image document then place a new image document in there so my picturebox will show the new image document.

int picWidth, picHeight;

private void Form1_Load(object sender, EventArgs e)
{
    timer1_Tick(sender, e);
}

private void File_Length()
{
    try
    {

        string path = @"C:\image\";
        string[] filename = Directory.GetFiles(path, "*.tif"); //gets a specific image doc.

        if (filename.Length > 0)
        {
            FileInfo fi = new FileInfo(filename[0]);
            byte[] buff = new byte[fi.Length];
            using (FileStream fs = File.OpenRead(filename[0]))
            {
                fs.Read(buff, 0, (int)fi.Length);
            }
            MemoryStream ms = new MemoryStream(buff);
            Bitmap img1 = new Bitmap(ms);
            //opened = true; // the files was opened.
            pictureBox1.Image = img1;

            pictureBox1.Width = img1.Width;
            pictureBox1.Height = img1.Height;
            picWidth = pictureBox1.Width;
            picHeight = pictureBox1.Height;
        }
    }
    catch(Exception)
    {

    }
}

public void InitTimer()
{
    timer1 = new Timer(); 
    timer1.Tick += new EventHandler(timer1_Tick); //calls method
    timer1.Interval = 2000; // in miliseconds (1 second = 1000 millisecond)
    timer1.Start(); //starts timer
}

private void timer1_Tick(object sender, EventArgs e)
{
    File_Length(); //checking the file length every 2000 miliseconds
}

Answer 1

Let's start here:

string[] filename = Directory.GetFiles(path, "*.tif");

If you know there can be only one file then you can try to get it right away with SingleOrDefault

var imageFileName = Directory.GetFiles(path, "*.tif").SingleOrDefault();

the condition will then become

var fileExists = !string.IsNullOrEmpty(imageFileName);
if (!fileExists)
{
   return;
}

---

Further you don't need all this code

FileInfo fi = new FileInfo(filename[0]);
byte[] buff = new byte[fi.Length];
using (FileStream fs = File.OpenRead(filename[0]))
{
  fs.Read(buff, 0, (int)fi.Length);
}
MemoryStream ms = new MemoryStream(buff);
Bitmap img1 = new Bitmap(ms);
//opened = true; // the files was opened.
pictureBox1.Image = img1;

instead you can just use

pictureBox1.Image = Image.FromFile(imageFileName);

---

besides there is a memory leak, you need to dispose the old image so before assigning a new one:

if (pictureBox.Image != null)
{
     pictureBox.Image.Dispose();
     pictureBox.Image = null;
}
pictureBox1.Image = Image.FromFile(imageFileName);

---

About the timer...

time1 = new Timer(); 
time1 .Tick += new EventHandler(timer1_Tick); //calls method
time1 .Interval = 2000; // in miliseconds (1 second = 1000 millisecond)
time1 .Start(); //starts timer

You have too many obvious comments here. There is no need to comment start as start ;-) One of them can even be replaced by a helper variable that makes the comment unnecessary. Meaningful variables are always better then comments.

const int oneSecondInMilisecods = 1000;

time1 = new Timer(); 
time1.Tick += timer1_Tick;
time1.Interval = 2 * oneSecondInMilisecods;
time1.Start();

Notice that you can also use a short event handler assignment.

---

Other suggestions:

• You should use full names instead of abbreviations like fi or buff, imageFileInfo and imageBytes sounds much better and you don't have to think first what those variables are for

• Collections should have plural form so string[] filename should be string[] imageFileNames

• File_Length is not a good name becuause it doesn't tell us anything about what the method does and in your case it does a completely different thing. LoadImage would be much better here, we also don't put _ in method names unless they are event handlers or tests.

• I would also change the name of the timer1 to loadImageTimer

• You didn't put all the image loading code inside the time1_Tick event handler which is good ;-)

Answer 2

The high CPU usage is probably because you are checking checking the folder and loading the image every 2 seconds. Instead of using a timer, there's a FileSystemWatcher class that allows you to handle events if the files in a specified directory have been changed, renamed, etc.

For your needs, you could do something like:

        FileSystemWatcher watcher = new FileSystemWatcher();
        watcher.Path = @"C:\image\";

        watcher.NotifyFilter = NotifyFilters.LastAccess | NotifyFilters.LastWrite
           | NotifyFilters.FileName | NotifyFilters.DirectoryName;

        watcher.Filter = "*.tif";

        watcher.Changed += new FileSystemEventHandler(File_Length);
        watcher.Created += new FileSystemEventHandler(File_Length);
        watcher.Deleted += new FileSystemEventHandler(File_Length);
        watcher.Renamed += new RenamedEventHandler(File_Length);

        watcher.EnableRaisingEvents = true;

This way, the picture is refreshed only if needed.