patterncsharpMinor
Displaying a new image document in Picturebox
Viewed 0 times
pictureboximagenewdisplayingdocument
Problem
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.
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
}Solution
Let's start here:
If you know there can be only one file then you can try to get it right away with
the condition will then become
Further you don't need all this code
instead you can just use
besides there is a memory leak, you need to dispose the old image so before assigning a new one:
About the 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.
Notice that you can also use a short event handler assignment.
Other suggestions:
-
You should use full names instead of abbreviations like
-
Collections should have plural form so
-
-
I would also change the name of the
-
You didn't put all the image loading code inside the
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
SingleOrDefaultvar 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 timerYou 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 ;-)Code Snippets
string[] filename = Directory.GetFiles(path, "*.tif");var imageFileName = Directory.GetFiles(path, "*.tif").SingleOrDefault();var fileExists = !string.IsNullOrEmpty(imageFileName);
if (!fileExists)
{
return;
}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.Image = Image.FromFile(imageFileName);Context
StackExchange Code Review Q#109922, answer score: 2
Revisions (0)
No revisions yet.