current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

In my ASP.NET form I have a file upload control where I am letting users upload image files, converting it into base64String to store it in the database.

I am using following code to do so,

     if (FileUploadControl.HasFile)
            {
                var img = new Bitmap(FileUploadControl.FileContent);
                if (img.Height > 133 || img.Width > 133)
                {
                    img = ResizeImage(img, 132, 132);
                }

                u.Photo = ImageToBase64String(img);
            }

    private static string ImageToBase64String(Bitmap img)
    {
        var ms = new System.IO.MemoryStream();
        img.Save(ms, System.Drawing.Imaging.ImageFormat.Jpeg);
        byte[] byteImage = ms.ToArray();
        var base64String = Convert.ToBase64String(byteImage);
        return base64String;
    }

    private static Bitmap ResizeImage(System.Drawing.Image image, int width, int height)
    {
        var destRect = new Rectangle(0, 0, width, height);
        var destImage = new Bitmap(width, height);

        destImage.SetResolution(image.HorizontalResolution, image.VerticalResolution);

        using (var graphics = Graphics.FromImage(destImage))
        {
            graphics.CompositingMode = CompositingMode.SourceCopy;
            graphics.CompositingQuality = CompositingQuality.HighQuality;
            graphics.InterpolationMode = InterpolationMode.HighQualityBicubic;
            graphics.SmoothingMode = SmoothingMode.HighQuality;
            graphics.PixelOffsetMode = PixelOffsetMode.HighQuality;

            using (var wrapMode = new ImageAttributes())
            {
                wrapMode.SetWrapMode(WrapMode.TileFlipXY);
                graphics.DrawImage(image, destRect, 0, 0, image.Width, image.Height, GraphicsUnit.Pixel, wrapMode);
            }
        }

        return destImage;
    }

Now I am worried about followings,

  • Overall code review
  • Creating two Bitmap classes they are disposable but am not sure if I can dispose them at all
  • I seem to be doing unnecessary conversion because I don't have enough knowledge.

Goal

  • Convert uploaded images to 132 x 132 if they are over it and save them as base64String.
share|improve this question
    
Do you have to convert the bitmap data to Base64? You can store it in the database in binary form, which will require less storage space, and which will avoid a cpu bound conversion to base 64 for a huge data like a bitmap – Oguz Ozgul yesterday

1 Answer 1

up vote 5 down vote

In ImageToBase64String() you do not dispose your stream. GC will do it but server-side is never never a good idea to postpone resource deallocation:

static string ImageToBase64String(Bitmap img)
{
    using (var ms = new System.IO.MemoryStream())
    {
        img.Save(ms, System.Drawing.Imaging.ImageFormat.Jpeg);
        return Convert.ToBase64String(ms.ToArray());
    }
}

Note that if you use this feature often you may reduce memory pressure reusing same buffer, you may implement an ObjectPool<MemoryStream> class.

You create a new bitmap just for resizing then yes, it has to be disposed too:

private static string ResizeAndConvertToBase64(Stream stream)
{
    using (var image = new Bitmap(FileUploadControl.FileContent))
    {
        if (IsTooBig(image))
        {
            using (var scaledImage = ScaleDownImage(image))
            {
                return ImageToBase64String(scaledImage);
            }
        }

        return ImageToBase64String(image);
    }
}

private static bool IsTooBig(Bitmap bitmap)
{
    return bitmap.Width > MaximumWidth || bitmap.Height > MaximumHeight;
}

Note I also moved magic numbers from code to constants MaximumWidth and MaximumHeight. Now you still have to deal with ScaleDownImage(). First of all you may want to keep image aspect ratio then I'd first move this calculation to another separate function (I dropped settings unused for image drawing):

private static Bitmap ScaleDownImage(Bitmap image)
{
    var newSize = ResolveScaledDownImageSize(image);
    var scaled = new Bitmap(newSize.Width, newSize.Height, PixelFormat.Format32bppArgb);

    using (var g = Graphics.FromImage(scaled))
    {
        g.CompositingQuality = CompositingQuality.HighQuality;
        g.InterpolationMode = InterpolationMode.HighQualityBicubic;
        g.SmoothingMode = SmoothingMode.AntiAlias;
        g.PixelOffsetMode = PixelOffsetMode.HighQuality;

        g.DrawImage(image, new Rectangle(Point.Empty, newSize));
    }

    return scaled;
}

There is also a built-in method to re-scale images, you should check if it satisfy your needing, in this case drop ScaleDownImage() and change ResizeAndConvertToBase64() as follow:

private static string ResizeAndConvertToBase64(Stream stream)
{
    using (var image = new Bitmap(FileUploadControl.FileContent))
    {
        if (IsTooBig(image))
        {
            using (var scaledImage = new Bitmap(image, ResolveScaledDownImageSize(image)))
            {
                return ImageToBase64String(scaledImage);
            }
        }

        return ImageToBase64String(image);
    }
}

In general don't forget to always call Dispose() for classes which implement IDisposable and whenever possible use using statement to simplify that code.

Now only ResolveScaledDownImageSize() function is empty. If you do not need to keep aspect ratio you may simply:

private static ResolveScaledDownImageSize(Bitmap image)
{
    return new Size(
        Math.Min(MaximumWidth, image.Width),
        Math.Min(MaximumHeight, image.Height));
}

To keep aspect ratio you may keep width or height (according to which one is bigger than required), you can use one of these to perform calculation:

\$height_{new} = \frac{height}{width} width_{new}\$

or

\$width_{new} = \frac{width}{height} height_{new}\$

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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