I have method which check for a recorded image name in a database. If there is such I try to load the image with the recorded path. If there isn't I load a default image.
First I had my whole method in a try-catch
block where catch(Exception ex)
and no matter what was the exception I just returned Error loading image
:
if (File.Exists(imgPath + "\\" + imageName))
{
try
{
using (var temp = new Bitmap(imgPath + "\\" + imageName))
{
pictureBox1.Image = new Bitmap(temp);
}
if (pictureBox1.Image.Width > defaultPicBoxWidth)
{
pictureBox1.Width = defaultPicBoxWidth;
}
}
catch (Exception ex)
{
logger.Error(ex.ToString());
MessageBox.Show("Error loading image!", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
}
}
Then I figured out that sometimes I may have a record in the database but for some reason the file might be missing so I add check for that :
if (imageName != null && !File.Exists(imgPath + "\\" + imageName))
Now I need a proper message for this case too. I get to the conclusion that I can either - use several try-catch
blocks to handle those parts or throw an exception and handle the exception from where the method is called.
I chose the second option and now the whole code is :
if (imageName != null && !File.Exists(imgPath + "\\" + imageName))
{
throw new FileNotFoundException();
}
if (File.Exists(imgPath + "\\" + imageName))
{
//try
//{
using (var temp = new Bitmap(imgPath + "\\" + imageName))
{
pictureBox1.Image = new Bitmap(temp);
}
if (pictureBox1.Image.Width > defaultPicBoxWidth)
{
pictureBox1.Width = defaultPicBoxWidth;
}
//}
//catch (Exception ex)
//{
// logger.Error(ex.ToString());
// MessageBox.Show("Error loading image!", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
//}
}
And this is where I call the method :
try
{
//The name of the method described above
LoadSavedOrDefaultImage(imageInfo, entity.Picture, txtCode.Text, imageLocation);
}
catch (FileNotFoundException ex)
{
logger.Error(ex.ToString());
MessageBox.Show("Error loading image! The file wasn't found.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
}
catch (Exception ex)
{
LogErrorAndShowMessage(ex, Resources.ERROR_LOAD);
}
I like this better but I can't say exactly why. The general question is - is this correct way to handle exceptions. And the more specific one - in my exact case where is the better place to put the try-catch
blocks? It makes sense to me to be in the body of the method itself because that way I won't need to write those try-catch
block everywhere I call the method for me it's even more encapsulated this way. Also now there are two try-catch
blocks but if in the future the logic changes I might want to throw more different exceptions, which is another reason to keep the exception handling in the method itself rather where it's called but on the other hand... I want to read your opinion.