Feedback on code to Serialize, Deserialize and Save Image

Question

Here is my code to Serialize, Deserialize and Save an image to the file system. I have looked at many examples of serialization/deserialization and I just want to get some feedback as I am sure my code could be improved. Any feedback would be greatly appreciated. I know this is a common problem so hopefully this question will be a good resource for others in the future.

This is the revised code using recommendations:

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

    private void RunTest()
    {
        byte[] jpgba = ConvertFileToByteArray("D:\\Images\\Image01.jpg");
        using (Image jpgimg = ConvertByteArrayToImage(jpgba))
        {
            SaveImageToFileSystem(jpgimg, "D:\\Images\\Image01_Copy.jpg");
        }

        byte[] pngba = ConvertFileToByteArray("D:\\Images\\Image02.png");
        using (Image pngimg = ConvertByteArrayToImage(pngba))
        {
            SaveImageToFileSystem(pngimg, "D:\\Images\\Image02_Copy.png");
        }

        byte[] gifba = ConvertFileToByteArray("D:\\Images\\Image03.gif");
        using (Image gifimg = ConvertByteArrayToImage(gifba))
        {
            SaveImageToFileSystem(gifimg, "D:\\Images\\Image03_Copy.gif");
        }

        MessageBox.Show("Test Complete");
        this.Close();
    }

    private static byte[] ConvertFileToByteArray(String FilePath)
    {
        return File.ReadAllBytes(FilePath);
    }

    private static Image ConvertByteArrayToImage(byte[] ImageByteArray)
    {
        using (MemoryStream ms = new MemoryStream(ImageByteArray))
        {
            return Image.FromStream(ms);
        }
    }

    private static void SaveImageToFileSystem(Image ImageObject, string FilePath)
    {
        // ImageObject.Save(FilePath, ImageObject.RawFormat);
        // This method only works with .png files.

        // This method works with .jpg, .png and .gif
        // Need to copy image before saving.
        using (Image img = new Bitmap(ImageObject.Width, ImageObject.Height))
        {
            using (Graphics tg = Graphics.FromImage(img))
            {
                tg.DrawImage(ImageObject, 0, 0);
            }
            img.Save(FilePath, img.RawFormat);
        }
        return;
    }

Answer 1

What I have see from quick look:

Streams should be wrapped in using(...) pattern, in your case if exception occurs during processing, then Dispose() won't be called.

using (FileStream fs = new FileStream(FilePath, FileMode.Open))
{
    // Another small optimization, removed unnecessary variable 
    byte[] iba = new byte[(int)fs.Length];
    fs.Read(iba, 0, iba.Length);
}

You should catch only exceptions you expect. For example in SerializeImage this will be IOException. Catching all exceptions is very bad practice.

}
catch (IOException ex)
{

Image.FromStream method depends on stream, so if you close underlying stream and return Image you can receive unpredictable behavior (well, in most cases this will work, but sometimes error occurs). So you need to create image copy and return it.

using (MemoryStream ms = new MemoryStream(ImageByteArray))
{
    using (Image img = Image.FromStream(ms))
    {
        return new Bitmap(img);
    }
}

You are not disposed tg graphics object and img object in SaveImage method (but disposed ImageObject, see next paragraph). And in general I do not see necessity in such logic, simply call ImageObject.Save(..., ImageFormat.Png) if you want to save image preserving quality.

In the same method (SaveImage) you are disposed ImageObject parameter. This is also bad practice in most cases, consider disposing this image outside worker method by using using(...) pattern.

Answer 2

Here's a bit more:

private void RunTest()
{
    // byte array that can be stored in DB
    byte[] iba;

    // image object to display in picturebox or used to save to file system.

    iba = ReadImage("D:\\Images\\Image01.jpg");
    using (Image img = DeserializeImage(iba))
    {
        SaveImage(img, "D:\\Images\\Image01_Copy.jpg");
    }

    iba = ReadImage("D:\\Images\\Image02.png");
    using (Image img1 = DeserializeImage(iba))
    {
        SaveImage(img1, "D:\\Images\\Image02_Copy.png");
    }

    iba = ReadImage("D:\\Images\\Image03.gif");
    using (var img2 = DeserializeImage(iba))
    {
        SaveImage(img2, "D:\\Images\\Image03_Copy.gif");
    }

    MessageBox.Show("Test Complete");
}

private static byte[] ReadImage(String filePath)
{
    // This seems to be the easiest way to serialize an image file
    // however it would be good to take a image object as an argument
    // in this method.
    using (var fs = new FileStream(filePath, FileMode.Open))
    {
        Int32 fslength = Convert.ToInt32(fs.Length);
        var iba = new byte[fslength];
        fs.Read(iba, 0, fslength);
        return iba;
    }
}

private static Image DeserializeImage(byte[] imageByteArray)
{
    using (var ms = new MemoryStream(imageByteArray))
    {
        return Image.FromStream(ms);
    }
}

private static void SaveImage(Image imageObject, string filePath)
{
    // I could only get this method to work for .png files.
    // imageObject.Save(filePath, imageObject.RawFormat);

    // This method works with .jpg, .png and .gif
    // Need to copy image before saving.
    using (Image img = new Bitmap(imageObject.Width, imageObject.Height))
    {
        using (Graphics tg = Graphics.FromImage(img))
        {
            tg.DrawImage(imageObject, 0, 0);
        }

        img.Save(filePath, img.RawFormat);
    }

    return;
}

Note what you called Serializing is just reading the bytes in. Serializing is more what you're doing when you Save.

I got rid of all the try/catch blocks. The best they were doing for you is telling you whether the problem happened in Reading, Saving or Deserializing. You can determine that from the stack trace, which you were destroying by only displaying ex.Message.

You were also returning null on a serious exception, propagating failure.

Beside that I agree with everything arbiter said.

Answer 3

As John Saunder says, serializing and deserializing are more than just reading the raw data from a file. See Wiki on Serialization

For images in .net, you don't need to use anything more than the provided framework methods (most of the time)

So Loading an Image (De-Serialization) in .net is.

using System.Drawing.Image;

Image test;

test = Image.FromFile(@"C:\myfile.jpg")
test = Image.FromStream(myStream); // or you can load from an existing stream

Likewise, Saving the image (Serialization) is:

test.Save(@"C:\anotherFile.jpg", System.Drawing.Imaging.ImageFormat.Jpeg);

---

These are the basics of loading and saving an image in .net. If you have a more specific scenario, ask another question.