12
\$\begingroup\$

How does this class to resize an image look?

using System;
using System.Collections.Generic;
using System.Web;
using System.Drawing;
using System.IO;

/*
 * Resizes an image
 **/
public static class ImageResizer
{
    // Saves the image to specific location, save location includes filename
    private static void saveImageToLocation(Image theImage, string saveLocation)
    {
        // Strip the file from the end of the dir
        string saveFolder = Path.GetDirectoryName(saveLocation);
        if (!Directory.Exists(saveFolder))
        {
            Directory.CreateDirectory(saveFolder);
        }
        // Save to disk
        theImage.Save(saveLocation);
    }

    // Resizes the image and saves it to disk.  Save as property is full path including file extension
    public static void resizeImageAndSave(Image ImageToResize, int newWidth, int maxHeight, bool onlyResizeIfWider, string thumbnailSaveAs)
    {
        Image thumbnail = resizeImage(ImageToResize, newWidth, maxHeight, onlyResizeIfWider);
        thumbnail.Save(thumbnailSaveAs);
    }
    // Overload if filepath is passed in
    public static void resizeImageAndSave(string imageLocation, int newWidth, int maxHeight, bool onlyResizeIfWider, string thumbnailSaveAs)
    {
        Image loadedImage = Image.FromFile(imageLocation);
        Image thumbnail = resizeImage(loadedImage, newWidth, maxHeight, onlyResizeIfWider);

        saveImageToLocation(thumbnail, thumbnailSaveAs);
    }

    // Returns the thumbnail image when an image object is passed in
    public static Image resizeImage(Image ImageToResize, int newWidth, int maxHeight, bool onlyResizeIfWider)
    {
        // Prevent using images internal thumbnail
        ImageToResize.RotateFlip(System.Drawing.RotateFlipType.Rotate180FlipNone);
        ImageToResize.RotateFlip(System.Drawing.RotateFlipType.Rotate180FlipNone);

        // Set new width if in bounds
        if (onlyResizeIfWider)
        {
            if (ImageToResize.Width <= newWidth)
            {
                newWidth = ImageToResize.Width;
            }
        }

        // Calculate new height
        int newHeight = ImageToResize.Height * newWidth / ImageToResize.Width;
        if (newHeight > maxHeight)
        {
            // Resize with height instead
            newWidth = ImageToResize.Width * maxHeight / ImageToResize.Height;
            newHeight = maxHeight;
        }

        // Create the new image
        Image resizedImage = ImageToResize.GetThumbnailImage(newWidth, newHeight, null, IntPtr.Zero);

        // Clear handle to original file so that we can overwrite it if necessary
        ImageToResize.Dispose();


        return resizedImage;
    }
    // Overload if file path is passed in instead
    public static Image resizeImage(string imageLocation, int newWidth, int maxHeight, bool onlyResizeIfWider)
    {
        Image loadedImage = Image.FromFile(imageLocation);
        return resizeImage(loadedImage, newWidth, maxHeight, onlyResizeIfWider);
    }
}
\$\endgroup\$
  • 3
    \$\begingroup\$ PascalCase on the methods...since you stated no matter how nitty... \$\endgroup\$ – Aaron McIver Feb 2 '11 at 14:46
  • \$\begingroup\$ Thanks, yeah I need to work on my naming convention for sure, I'm just so used to leading lower \$\endgroup\$ – Tom Gullen Feb 2 '11 at 14:47
12
\$\begingroup\$

PascalCase the method names and method params if you are feeling overly ambitious.

    // Set new width if in bounds
    if (onlyResizeIfWider)
    {
        if (ImageToResize.Width <= newWidth)
        {
            newWidth = ImageToResize.Width;
        }
    }

FindBugs barks in Java for the above behavior... refactor into a single if since you are not doing anything within the first if anyways...

    // Set new width if in bounds
    if (onlyResizeIfWider && ImageToResize.Width <= newWidth)
    {
        newWidth = ImageToResize.Width;
    }

Comments here could be a bit more descriptive; while you state what the end result is I am still lost as to why that would resolve the issue.

    // Prevent using images internal thumbnail
    ImageToResize.RotateFlip(System.Drawing.RotateFlipType.Rotate180FlipNone);
    ImageToResize.RotateFlip(System.Drawing.RotateFlipType.Rotate180FlipNone); 

Maybe something similar to what is stated on this blog...

    // Prevent using images internal thumbnail since we scale above 200px; flipping
    // the image twice we get a new image identical to the original one but without the    
    // embedded thumbnail
    ImageToResize.RotateFlip(System.Drawing.RotateFlipType.Rotate180FlipNone);
    ImageToResize.RotateFlip(System.Drawing.RotateFlipType.Rotate180FlipNone);
\$\endgroup\$
  • 1
    \$\begingroup\$ I wouldn't recommend pascal casing parameters even in an ambitious scenario as I've never once seen it in any common commercial or BCL code and the purpose of class and variable/param casing being different is to identify definitions from declarations. Due to compiler optimizations and condition short-circuiting both variations of the "if (onlyResize..." will evaluate to the same native code so, while I hold the same opinion on combining them, there are circumstances where readability or comparison to requirements may benefit and there's no harm either way. \$\endgroup\$ – TheXenocide Feb 2 '11 at 15:14
8
\$\begingroup\$

If you're using C# 3.0, you can use extension methods

// declare as
public static Image ResizeImage(this Image source, ...

// use as 
Image myThumb = myImage.Resize(...);

Treating width differently than height seems inconsistent.

Never dispose passed-in arguments in a public function (ImageToResize). The caller almost never expects this to happen.

Do dispose the temporary local variables (loadedImage). No other code can do it, and it could cause a memory leak.

\$\endgroup\$
5
\$\begingroup\$

In C# it's generally common practice to use Pascal Case in method names (so SaveImageToLocation instead of saveImageToLocation) and Camel Case in parameter names (so "public static Image ResizeImage(Image imageToResize, ...")

RotateFlip can be a rather expensive operation just to clear an internal thumbnail. As far as the images are concerned, do you need to support vector images or will this generally be used for Bitmap (rasterized) images (this includes compressed variations like png, jpg, gif, etc.)? If you only plan to output Bitmaps then I suggest using the Bitmap(Image original, int width, int height) constructor which will take a source image and scale it, removing the need to do costly rotations. There are a number of methods to draw scaled images, some of which are much more efficient than others and each have varying pros and cons to using them, but the biggest advantage to GetThumbnailImage is use of embedded thumbnails.

It is generally not good practice to dispose of a parameter so it may warrant a different pattern (returning an image and letting the calling code call image.Save(filename) at its own discretion isn't that terrible), but if you intend to leave it this way you should definitely comment it. Refer to this post for information about loading images without locking files. The overloads that receive a file path instead of an Image object should wrap their loaded Image files in a using block (or try/finally+dispose) like so:

public static void ResizeImageAndSave(string imageLocation, int newWidth, int maxHeight, bool onlyResizeIfWider, string thumbnailSaveAs)
{
    Image thumbnail = null;

    try
    {
        using (Image loadedImage = Image.FromFile(imageLocation)) 
        {
            thumbnail = resizeImage(loadedImage, newWidth, maxHeight, onlyResizeIfWider);
        }
        saveImageToLocation(thumbnail, thumbnailSaveAs);
    }
    finally
    {
        if (thumbnail != null) thumbnail.Dispose();
    }
}

Hope this helps :)

\$\endgroup\$
5
\$\begingroup\$

You should be using the using statement to dispose of types which implement IDisposable, such as Image in this case:

using System;
using System.Drawing;
using System.IO;

/*
 * Resizes an image
 **/
public static class ImageResizer
{
    // Saves the image to specific location, save location includes filename
    private static void saveImageToLocation(Image theImage, string saveLocation)
    {
        // Strip the file from the end of the dir
        string saveFolder = Path.GetDirectoryName(saveLocation);
        if (!Directory.Exists(saveFolder))
        {
            Directory.CreateDirectory(saveFolder);
        }
        // Save to disk
        theImage.Save(saveLocation);
    }

    // Resizes the image and saves it to disk.  Save as property is full path including file extension
    public static void resizeImageAndSave(Image ImageToResize, int newWidth, int maxHeight, bool onlyResizeIfWider, string thumbnailSaveAs)
    {
        using (Image thumbnail = resizeImage(ImageToResize, newWidth, maxHeight, onlyResizeIfWider))
        {
            thumbnail.Save(thumbnailSaveAs);
        }
    }
    // Overload if filepath is passed in
    public static void resizeImageAndSave(string imageLocation, int newWidth, int maxHeight, bool onlyResizeIfWider, string thumbnailSaveAs)
    {
        using (Image loadedImage = Image.FromFile(imageLocation))
        using (Image thumbnail = resizeImage(loadedImage, newWidth, maxHeight, onlyResizeIfWider))
        {
            saveImageToLocation(thumbnail, thumbnailSaveAs);
        }
    }

    // Returns the thumbnail image when an image object is passed in
    public static Image resizeImage(Image ImageToResize, int newWidth, int maxHeight, bool onlyResizeIfWider)
    {
        // Prevent using images internal thumbnail
        ImageToResize.RotateFlip(RotateFlipType.Rotate180FlipNone);
        ImageToResize.RotateFlip(RotateFlipType.Rotate180FlipNone);

        // Set new width if in bounds
        if (onlyResizeIfWider)
        {
            if (ImageToResize.Width <= newWidth)
            {
                newWidth = ImageToResize.Width;
            }
        }

        // Calculate new height
        int newHeight = ImageToResize.Height * newWidth / ImageToResize.Width;
        if (newHeight > maxHeight)
        {
            // Resize with height instead
            newWidth = ImageToResize.Width * maxHeight / ImageToResize.Height;
            newHeight = maxHeight;
        }

        // Create the new image
        Image resizedImage = ImageToResize.GetThumbnailImage(newWidth, newHeight, null, IntPtr.Zero);

        // Clear handle to original file so that we can overwrite it if necessary
        // Note from Jesse C. Slicer: I wouldn't do this here - let the calling code Dispose it.
        ////ImageToResize.Dispose();


        return resizedImage;
    }
    // Overload if file path is passed in instead
    public static Image resizeImage(string imageLocation, int newWidth, int maxHeight, bool onlyResizeIfWider)
    {
        using (Image loadedImage = Image.FromFile(imageLocation))
        {
            return resizeImage(loadedImage, newWidth, maxHeight, onlyResizeIfWider);
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

Some observations about the abstractions represented.

The main abstraction that has been coded is not general image resizing but rather custom thumbnail creation. The thumbnail idea appears several times in variable names and comments, plus the code logic supports this as well. I suggest renaming classes and methods to surface this intent more clearly. Also, I suggest separating into two classes the concerns that operate on an image from those that operate on an image file. The simple "better thumbnailer" makes a nice extension method where it will appear (both in intellisense and logically) alongside the built-in Image.GetThumbnailImage method.

I did not change any logic (except for the last method in my answer since the orig seemed broken), though I did remove a few unhelpful comments (which clutter rather than illuminate, and some are just wrong, like referring to non-existent "save as" property in the static class). (I find that focusing on getting the abstractions right and using descriptive class and method names makes most comments unnecessary.)

I don't know why the build-in thumbnail-creation ability of Image is insufficient, so without more knowledge at the moment I named "your" thumbnailer extension method CreateBetterThumbnail to distinguish it. It is a poor name, so I would suggest something more representative of your intent like CreateTinyThumbnail or CreateMonochromeThumbnail or whatnot (where those names are just examples to illustrate a point; I know you are not trying to do either of those things).

public static class ImageExtensions
{
   public static Image CreateBetterThumbnail(this Image ImageToThumbnail, int newWidth, int maxHeight, bool onlyResizeIfWider)
   {
      // **Should make copy of incoming image if we are going to mess with it**

      // Prevent using images internal thumbnail
      // **I'd like to see a comment about why this works or is important**
      ImageToThumbnail.RotateFlip(System.Drawing.RotateFlipType.Rotate180FlipNone);
      ImageToThumbnail.RotateFlip(System.Drawing.RotateFlipType.Rotate180FlipNone);

      // Set new width if in bounds
      if (onlyResizeIfWider)
      {
         if (ImageToThumbnail.Width <= newWidth)
         {
            newWidth = ImageToThumbnail.Width;
         }
      }

      // Calculate new height
      int newHeight = ImageToThumbnail.Height * newWidth / ImageToThumbnail.Width;
      if (newHeight > maxHeight)
      {
         // Resize with height instead
         newWidth = ImageToThumbnail.Width * maxHeight / ImageToThumbnail.Height;
         newHeight = maxHeight;
      }

      Image thumbnail = ImageToThumbnail.GetThumbnailImage(newWidth, newHeight, null, IntPtr.Zero);

      // Clear handle to original file so that we can overwrite it if necessary
      ImageToThumbnail.Dispose();

      return thumbnail;
   }
}

public static class ThumbnailFileCreator
{
   // Saves the image to specific location, save location includes filename
   private static void saveImageToLocation(Image theImage, string saveLocation)
   {
      // Strip the file from the end of the dir
      string saveFolder = Path.GetDirectoryName(saveLocation);
      if (!Directory.Exists(saveFolder))
      {
         Directory.CreateDirectory(saveFolder);
      }
      // Save to disk
      theImage.Save(saveLocation);
   }

   public static void CreateThumbnailAndSave(Image ImageToThumbnail, int newWidth, int maxHeight, bool onlyResizeIfWider, string thumbnailSaveAs)
   {
      Image thumbnail = ImageToThumbnail.CreateBetterThumbnail(newWidth, maxHeight, onlyResizeIfWider);
      thumbnail.Save(thumbnailSaveAs);
   }

   public static void CreateThumbnailAndSave(string imageLocation, int newWidth, int maxHeight, bool onlyResizeIfWider, string thumbnailSaveAs)
   {
      Image loadedImage = Image.FromFile(imageLocation);

      CreateThumbnailAndSave(loadedImage, newWidth, maxHeight, onlyResizeIfWider, thumbnailSaveAs);
   }
}

Hope this is helpful!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.