patterncppModerate
Refactoring image loading/manipulation class
Viewed 0 times
refactoringimagemanipulationloadingclass
Problem
I have this
projects and games I eventually write. It was written in C++98 style and it
is starting to show its age. Over time, I added a few C++11 elements here and there,
but I think it is now due for a major refactoring.
image.hpp:
```
#ifndef IMAGE_HPP
#define IMAGE_HPP
// Dependencies:
#include
#include
// ======================================================
// PixelFormat:
// ======================================================
//
// Supported internal image and texture formats.
//
struct PixelFormat
{
enum Enum
{
// RGB images are composed of 3 color components.
// Each component can be a byte [0,255] or a normalized float [0,1].
RgbU8,
RgbF32,
// RGBA images are composed of 3 color components and
// 1 component for the transparency level (alpha).
// Each component can be a byte [0,255] or a normalized float [0,1].
RgbaU8,
RgbaF32,
// Sentinel value. Not a valid pixel format. Internal use.
Invalid = -1
};
// Converts a pixel format enum value to a printable string, for debugging purposes.
static const char * toString(PixelFormat::Enum pixelFormat);
// Returns the size in bytes of a single pixel of a given pixel format.
static size_t sizeBytes(PixelFormat::Enum pixelFormat);
// Returns the number of color components of a pixel format.
static uint32_t componentCount(PixelFormat::Enum pixelFormat);
};
// ======================================================
// TPixel templates:
// ======================================================
// RGB pixel:
template
struct TPixel3
{
T r;
T g;
T b;
};
// RGBA pixel:
template
struct TPixel4
{
T r;
T g;
T b;
T a;
};
// ======================================================
// Image:
// ======================================================
//
// Represents and array of pixels.
// Images ar
Image class that I've been using for some time now on personalprojects and games I eventually write. It was written in C++98 style and it
is starting to show its age. Over time, I added a few C++11 elements here and there,
but I think it is now due for a major refactoring.
image.hpp:
```
#ifndef IMAGE_HPP
#define IMAGE_HPP
// Dependencies:
#include
#include
// ======================================================
// PixelFormat:
// ======================================================
//
// Supported internal image and texture formats.
//
struct PixelFormat
{
enum Enum
{
// RGB images are composed of 3 color components.
// Each component can be a byte [0,255] or a normalized float [0,1].
RgbU8,
RgbF32,
// RGBA images are composed of 3 color components and
// 1 component for the transparency level (alpha).
// Each component can be a byte [0,255] or a normalized float [0,1].
RgbaU8,
RgbaF32,
// Sentinel value. Not a valid pixel format. Internal use.
Invalid = -1
};
// Converts a pixel format enum value to a printable string, for debugging purposes.
static const char * toString(PixelFormat::Enum pixelFormat);
// Returns the size in bytes of a single pixel of a given pixel format.
static size_t sizeBytes(PixelFormat::Enum pixelFormat);
// Returns the number of color components of a pixel format.
static uint32_t componentCount(PixelFormat::Enum pixelFormat);
};
// ======================================================
// TPixel templates:
// ======================================================
// RGB pixel:
template
struct TPixel3
{
T r;
T g;
T b;
};
// RGBA pixel:
template
struct TPixel4
{
T r;
T g;
T b;
T a;
};
// ======================================================
// Image:
// ======================================================
//
// Represents and array of pixels.
// Images ar
Solution
If this code is working. Then DON'T do any major refactoring without unit tests.
You can add your standard modernizations that are easy to show work:
Use Copy and Swap Idiom on the assignment operator:
Easy re-write:
You could also add Move Semantics. (I did this to some old code and got a 30% boost in speed by doing nothing else but recompiling with C++11 on).
You can add your standard modernizations that are easy to show work:
Use Copy and Swap Idiom on the assignment operator:
Image & Image::operator = (const Image & other)
{
freeImageStorage(); // This is not exception safe.
initFromCopy(other); // If the initFromCopy() throws then you are in a bad position.
return *this;
}Easy re-write:
Image & Image::operator = (Image copy)
{
copy.swap(*this);
return *this;
}
void swap(Image& other) noexcept
{
using std::swap;
swap(data, other.data);
swap(dataSizeBytes, other.dataSizeBytes);
swap(width, other.width);
swap(height, other.height);
swap(pixelFormat, other.pixelFormat);
}You could also add Move Semantics. (I did this to some old code and got a 30% boost in speed by doing nothing else but recompiling with C++11 on).
Image(Image&& other) noexcept
: data(null)
{
other.swap(*this);
}
Image& Image::operator=(Image&& other) noexcept
{
other.swap(*this);
return *this;
}Code Snippets
Image & Image::operator = (const Image & other)
{
freeImageStorage(); // This is not exception safe.
initFromCopy(other); // If the initFromCopy() throws then you are in a bad position.
return *this;
}Image & Image::operator = (Image copy)
{
copy.swap(*this);
return *this;
}
void swap(Image& other) noexcept
{
using std::swap;
swap(data, other.data);
swap(dataSizeBytes, other.dataSizeBytes);
swap(width, other.width);
swap(height, other.height);
swap(pixelFormat, other.pixelFormat);
}Image(Image&& other) noexcept
: data(null)
{
other.swap(*this);
}
Image& Image::operator=(Image&& other) noexcept
{
other.swap(*this);
return *this;
}Context
StackExchange Code Review Q#67843, answer score: 10
Revisions (0)
No revisions yet.