We have a image utility method tightly coupled to the Sanselan library, and now I need to add a second library (Metadata Extractor) to try and read the image's metainfo in case Sanselan can't do it.
I say that our code is tightly coupled to Sanselan because we have several calls directly to Sanselan's static methods or to getters of an object return by one of these methods (Lines commented with // Sanselan)
BufferedImage toBufferedImageJpeg(byte[] image) throws ImageReadException, IOException
{
ImageInfo imageInfo = Sanselan.getImageInfo(image); // Sanselan
if (isImageTooBig(imageInfo.getHeight(), imageInfo.getWidth())) // Sanselan
{
throw new IllegalArgumentException("Image is too big");
}
if (imageInfo.getFormat().equals(ImageFormat.IMAGE_FORMAT_JPEG)) // Sanselan
{
if (ImageInfo.COLOR_TYPE_CMYK == imageInfo.getColorType()) // Sanselan
{
ICC_Profile iccProfile = Sanselan.getICCProfile(image); // Sanselan
ImageInputStream imageInputStream = ImageIO.createImageInputStream(new ByteArrayInputStream(image));
try
{
Iterator<ImageReader> readers = ImageIO.getImageReaders(imageInputStream);
if (false == readers.hasNext())
{
return null;
}
ImageReader reader = readers.next();
reader.setInput(imageInputStream);
WritableRaster raster = (WritableRaster) reader.readRaster(0, null);
return convertIccToRgb(raster, iccProfile);
}
catch (Exception e)
{
return null;
}
finally
{
IOUtils.closeQuietly(imageInputStream);
}
}
else
{
JPEGImageDecoder decoder = JPEGCodec.createJPEGDecoder(new ByteArrayInputStream(image));
return decoder.decodeAsBufferedImage();
}
}
else
{
throw new IllegalArgumentException("Not a jpeg image");
}
}
I want to change the above code as little as possible, for some reasons. Ideally I want just to replace the calls to Sanselan with calls to my new class:
BufferedImage toBufferedImageJpeg2(byte[] image) throws Exception
{
ImageMetaInfo imageInfo = new ImageMetaInfo(image);
if (isImageTooBig(imageInfo.getHeight(), imageInfo.getWidth())) // Sanselan
{
throw new IllegalArgumentException("Image is too big");
}
if (imageInfo.getFormat().equals(ImageFormat.IMAGE_FORMAT_JPEG)) // Sanselan
{
if (ImageInfo.COLOR_TYPE_CMYK == imageInfo.getColorType()) // Sanselan
{
ICC_Profile iccProfile = imageInfo.getICCProfile(); // Sanselan
ImageInputStream imageInputStream = ImageIO.createImageInputStream(new ByteArrayInputStream(image));
My approach is to create an object that provides the same methods that we're currently invoking (getHeight, getWidth, getColorType, getIccProfile, getFormat). I'll pass the byte [] to the object constructor, and define two strategies (not really sure if should be calling them strategies). The primary strategy will try to read the image using Sanselan. If it throws an exception, the failover strategy will try to read the image using the new library, Metadata Extractor.
public class ImageMetaInfo
{
public static final String IMAGE_FORMAT_JPEG = "JPEG";
private static Log LOGGER = LogFactory.getLog(ImageValidator.class);
private final int width;
private final int height;
private final String format;
private final int colorType;
private final ICC_Profile icc_profile;
public ImageMetaInfo(byte[] image) throws Exception
{
ImageMetaInfoWrapper wrapper;
try
{
wrapper = new SanselanImageMetaInfoWrapper();
}
catch (ImageReadException | IOException e)
{
LOGGER.error("Image could not be read with Sanselan, trying Metadata Extractor");
try
{
wrapper = new MetadataExtractorMetaInfoWrapper();
}
catch (ImageProcessingException | IOException | MetadataException e1)
{
LOGGER.error("Also failed to read the image using Metadata Extractor", e1);
throw new Exception("Couldn't create instance from the given data", e1);
}
}
this.width = wrapper.getWidth();
this.height = wrapper.getHeight();
this.format = wrapper.getFormat();
this.icc_profile = wrapper.getICCProfile();
this.colorType = wrapper.getColorType();
}
public int getWidth()
{
return width;
}
public int getHeight()
{
return height;
}
public Object getFormat()
{
return format;
}
public int getColorType()
{
return colorType;
}
public ICC_Profile getICCProfile()
{
return icc_profile;
}
}
And then, inside each implementation of ImageMetaInfoWrapper, parse the image with the corresponding library.
My problem is that this looks really too complicated. I have a wrapper (ImageMetaInfoWrapper) inside another wrapper (ImageMetaInfo).
- Is there a better/cleaner approach for this, keeping the changes to the existing code to a minimum?
- Can/should the Strategy Pattern be used here?
I thought about posting the question in Code Review, but I think that my question is on a higher level, more about design that coding.
