views:

264

answers:

6

I have a requirement to save an image file in various formats. The list of formats is subject to change quite often, so I wish the saving would be very extensible. Also, the saving can take place in many places (hard disk, ftp, http, etc). The list of saving locations is going to change very often too.

I thought I would use a base Image class and many deriving classes for each format:

ImageBase {}
JpegImage : ImageBase {}
TiffImage : ImageBase{}

and handle saving in each subclass appropriately to format. Is this a good design desicion?

Also, how can I attach an extensible Saving location mechanism (Ftp, file share, etc)?

I would like something like this:

var image=ImageBase.GetImageFromDisk(path);
//some casting to subclass maybe??
var tiffImage=image as TiffImage;
tiffImage.Location=new FtpLocation();//not sure if this is a good idea
tiffImage.Save();

The problem here, is that concrete image implementation should not know or care about the saving location. When calling Save(); on image subclass, I would like to delegate the work to some class, like FtpLocation.

Please advice how to put the pieces together.

Thank you.

Valentin.

+5  A: 

Firstly I would implement streams on your Image. This way you can create a constructor from a stream, and a method to create a stream to any of your "image" subclasses.

Additionally, I would create your "saving" infrastructure to simply accept streams and write them down onto your appropriate techniques (ftp, file etc.)

That way you end up with extensible images (if you can get a stream to it you can do many, many things) and you end up with a saving infrastructure that is extensible as well (anything that can go to a stream can be saved)

EDIT: Personally, a saving method on a data type object sounds like it's in the wrong place, but without knowing your entire system I can't say that for sure. Just my 2c.

Spence
+1  A: 

It seems to me that the concrete class should only deal with the raw data; this could be writing a local file (that is then handled by the base code), or to a stream.

For example, perhaps something like:

public void Save()
{
    // TODO: add any language-specific constructs like "using", etc
    Stream stream = Location.OpenWrite();
    Save(stream);
    stream.Close();
}

protected abstract void Save(Stream stream);

So the Location is responsible for providing a stream (that could be a memory stream, a transport stream, a temp file stream, etc), and optionally doing extra work when that stream is closed (by encapsulating an inner-stream via a decorator pattern). All the subclass does is write to a stream.

Loading is a bit trickier, since the base-class must presumably (given your suggested static load from the base-class) peek at the stream to identify the type. But ultimately, you could have something similar:

public static ImageBase Load(Location location)
{
    // TODO: add any language-specific constructs like "using", etc
    Stream stream = location.OpenRead();
    // TODO: wrap in a buffered/seekable stream so we can peek
    // TODO: parse headers and resolve image type
    ImageBase image = ...
    image.Location = location;
    stream.Position = 0; // rewind buffered/seekable stream
    // (don't use Load() since we have already opened the stream)
    image.Load(stream);
    stream.Close();
    return image;
}
protected abstract void Load(Stream stream);
public void Load()
{
    // TODO: add any language-specific constructs like "using", etc
    Stream stream = Location.OpenRead();
    Load(stream); // don't need to buffer if loading from subclass
    stream.Close();
}
Marc Gravell
A: 

Implement Save() in the ImageBase class, and not in the derived classes.

Igor Oks
A: 

I think that subclassing image itself for Saving/Loading is not a right idea, since you will not have instance to call Load on. Also image in memory is quite independent from the original format -- it is absolutely possible to open jpg and then save it as png, for example.

I would do it the following way:

Image

ImageFormat { Save(Image, Stream); Image Load(Stream); }
JpegFormat : ImageFormat {}
TiffFormat : ImageFormat {}

Now for the location you just provide a way to get a reading stream and writing stream.

Andrey Shchekin
+1  A: 

I’d go a slightly different way. (Syntax is Java.)

public class Image {
    public void load(byte[] imageData);
    public byte[] getImageData();
}

public class JpegImage extends Image {
    public void load(byte[] imageData) {
        /* decode image data. */
    }

    public byte[] getImageData() {
        /* encode and return the JPG data. */
    }
}

public class Location {
    public Image loadImage(String uri);
    public void saveImage(Image image);
}

public class HttpLocation extends Location {
    public Image loadImage(String uri) {
        byte[] = getData(uri);
        if (type == JPEG) {
            return new JpegImage().load(byte);
        } else if (type == PNG) {
            return new PngImage().load(byte);
        }
    }
    public void saveImage(Image image) {
        byte[] imageData = image.getImageData();
        /* upload. */
    }
}

The mapping from the web server’s content type to an Image class could also happen in a more reusable way in the Location base class (or a completely different helper class) but that’s about how I would do it.

Bombe
A: 

For the images , I would go with inheritance . For the loading/saving part , I'd go with the strategy design pattern , so that each image can have a LoadingAlgorithm and a SavingAlgorithm . This way you could vary both the loading and the saving process and don't have to make the load and save methods of the Image class .

Geo