Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I'm writing an app that will have an Image entity, and I'm already having trouble deciding whose responsibility each task should be.

First I have the Image class. It has a path, width, and other attributes.

Then I created an ImageRepository class, for retrieving images with a single and tested method, eg: findAllImagesWithoutThumbnail().

But now I also need to be able to createThumbnail(). Who should deal with that? I was thinking about having an ImageManager class, which would be an app-specific class (there would also be a third party image manipulation reusable component of choice, I'm not reinventing the wheel).

Or maybe it would be 0K to let the Image resize itself? Or let the ImageRepository and ImageManager be the same class?

What do you think?

share|improve this question
    
What does Image do so far? –  Winston Ewert Aug 31 '12 at 2:50
    
How do you expect to resize the images? Are you only ever shrinking images or can you imagine wanting to go back to the full size? –  Steven Burnap Aug 31 '12 at 3:05
    
@WinstonEwert It generates data like the formatted resolution (eg: the string '1440x900'), different URL's, and lets you modify some stats like 'views' or 'votes'. –  ChocoDeveloper Aug 31 '12 at 3:05
    
@StevenBurnap I consider the original image the most important thing, I only use thumbnails for faster browsing or if the user wants to resize to fit his desktop or something, so they are a separate thing. –  ChocoDeveloper Aug 31 '12 at 3:08
    
I would make the Image class immutable so that you don't have to defensively copy it when you pass it around. –  dan_waterworth Aug 31 '12 at 7:31

10 Answers 10

The question as asked is too vague to have a real answer as it really depends on how the Image objects are going to be used.

If you are only using the image at one size, and are resizing because the source image is the wrong size, it might be best to have the read code do the resizing. Have your createImage method take a width/height and then return the image having been resized at that width/height.

If you need multiple sizes, and if memory isn't a concern, it is best to keep the image in memory as originally read and do any resizing at display time. In such a case, there are a couple different designs you might use. The image width and height would be fixed, but you'd either have a display method that took a position and a target height/width or you'd have some method taking a width/height that returned an object that whatever display system you are using. Most drawing APIs I've used let you specify target size at draw time.

If the requirements cause for drawing at a different sizes often enough for performance to be a concern, you could have a method that creates a new image of a different size based on the original. An alternative would be to have your Image class cache different representations internally so that the first time you call display with the thumbnail size it does the resizing while the second time it just draws the cached copy it saved from last time. This uses more memory, but it is rare you'll have more than a few common resizings.

Another alternative is to have a single Image class that retains the base image and have that class contain one or more representations. An Image itself wouldn't have a height/width. Instead, it would start with one ImageRepresentation that had a height and width. You'd draw this representation. To "resize" an image, you'd ask the Image for a representation with certain height/width metrics. This would cause it to then contain this new representation as well as the original. This gives you a lot of control over exactly what is hanging around in memory at the cost of extra complexity.

I personally dislike classes that contain the word Manager because "manager" is a very vague word that doesn't really tell you much about exactly what the class does. Does it manage object lifetime? Does it stand between the rest of the application and the thing it manages?

share|improve this answer
    
"it really depends on how the Image objects are going to be used." This statement isn't really in line with the notion of encapsulation and loose coupling (although it may be taking the principle a little too far in this case). A better point of differentiation would be whether and Image's state meaningfully includes size or not. –  Chris Bye Oct 3 '12 at 17:49
    
Seems you are talking about a Image Editing application point of view. Not entirely clear if OP wanted this or not huh? –  andho Oct 25 '12 at 11:06

Keep things simple as long as the requirements are low, and improve your design later when you have to. I guess in most real-world cases there is nothing wrong to start with a design like that:

class Image
{
    public Image createThumbnail(int sizeX, int sizeY)
    {
         // ...
         // delegate the actual resize operation to a separate component
    }
}

Things may become different when you need to pass more parameters to createThumbnail(), and that parameters need a lifetime of their own. That will mean that you move the createThumbnail to another class, for example, a manager class, or an ImageResizer class, where those parameters are passed in by the constructor and so are bound to the lifetime of the ImageResizer object. Actually, I would start with the first approach and refactor later when I really need it.

share|improve this answer
    
well, if I'm delegating the call to a separate component already why not make it a responsibility of a ImageResizer class in the first place? When do you delegate a call instead of moving the responsibility to a new class? –  Songo Aug 31 '12 at 15:51
2  
@Songo: 2 possible reasons: (1) the code for delegating to the - perhaps already existing - separate component is not just a simple one-liner, perhaps just a sequence of 4 to 6 commands, thus you need a place to store it. (2) Syntactical sugar / ease of use: it will be just very handsome to write Image thumbnail = img.createThumbnail(x,y). –  Doc Brown Aug 31 '12 at 16:01
    
+1 aah I see. Thanks for the explanation :) –  Songo Aug 31 '12 at 16:26

I think it would have to be part of the Image class, as resizing in an external class would require the resizer to know the implementation of the Image, thereby violating encapsulation. I'm assuming Image is a base class, and you'll wind up with separate subclasses for concrete image types (PNG, JPEG, SVG, etc). Therefore, you'll either have to have corresponding resizing classes, or a generic resizer with a switch statement that resizes based on the implementation class -- a classic design smell.

One approach might be to have your Image constructor take a resource containing the image, and height and width parameters and create itself appropriately. Then resizing could be as simple as creating a new object using the original resource (cached inside the image) and the new size parameters. E.g. foo.createThumbnail() would simply return new Image(this.source, 250, 250). (with Image being the concrete type of foo, of course). This keeps your images immutable and their implementations private.

share|improve this answer
    
I like this solution, but I don't understand why you say that the resizer needs to know the internal implementation of Image. All it needs is the source and the target dimensions. –  ChocoDeveloper Aug 31 '12 at 20:36

I know that OOP is about encapsulating data and behavior together, but I don't think it's a good idea for an Image to have the resize logic embedded in this case, because an Image doesn't need to know how to resize itself to be an Image.

A thumbnail is actually a different Image. Perhaps you might have a datastructure that holds the relationship between a Photograph and it's Thumbnail (both of which are Images).

I try to divide my programs into things (like Images, Photographs, Thumbnails, etc.) and Services (like PhotographRepository, ThumbnailGenerator, etc.). Get your data structures right, and then define the services that let you create, manipulate, transform, persist and recover those data structures. I don't put any more behavior in my data structures than making sure they're created properly and used appropriately.

Therefore, no, an Image shouldn't contain the logic about how to make a Thumbnail. There should be a ThumbnailGenerator service that has a method like:

Image GenerateThumbnailFrom(Image someImage);

My bigger data structure might look like this:

class Photograph : Image
{
    public Photograph(Image thumbnail)
    {
        if(thumbnail == null) throw new ArgumentNullException("thumbnail");
        this.Thumbnail = thumbnail;
    }

    public Image Thumbnail { get; private set; }
}

Of course that might mean you're doing effort you don't want to do while constructing the object, so I would consider something like this OK too:

class Photograph : Image
{
    private Image thumbnail = null;
    private readonly Func<Image,Image> generateThumbnail;

    public Photograph(Func<Image,Image> generateThumbnail)
    {
        this.generateThumbnail = generateThumbnail;
    }


    public Image Thumbnail
    {
        get
        {
            if(this.thumbnail == null)
            {
                this.thumbnail = this.generateThumbnail(this);
            }
            return this.thumbnail;
        }
    }
}

... in the case where you want a data structure with lazy evaluation. (Sorry I didn't include my null checks and I didn't make it thread-safe, which is something you'd want if you were trying to mimic an immutable data structure).

As you can see, either of these classes is being built by some kind of PhotographRepository, which probably has a reference to a ThumbnailGenerator that it got through dependency injection.

share|improve this answer
    
I've been told I shouldn't create classes with no behavior. Not sure if when you say 'data structures' you are referring to the classes or something from C++ (is that this language?). The only data structures I know and use are the primitives. The services and DI part is spot on, I might end up doing this. –  ChocoDeveloper Aug 31 '12 at 20:42
    
@ChocoDeveloper: occasionally classes with no behavior are useful or necessary, depending on situation. Those are called value classes. Normal OOP classes are classes with hard-coded behavior. Composable OOP classes also have hard-coded behavior, but their structure of their composition can give rise to many behaviors needed by a software application. –  rwong Sep 1 '12 at 7:15

You have identified a single functionality you want to implement, so why shouldn't it be separate from everything you have identified so far? That is what the Single Responsibility Principle would suggest is the solution.

Create an IImageResizer interface that allows you to pass in an image and a target size, and which returns a new image. Then create an implementation of that interface. There are actually tons of ways to resize images, so you could even end up with more than one!

share|improve this answer
    
+1 for relevant SRP, but I'm not implementing the actual resizing, that's already delegated to a third party library as stated. –  ChocoDeveloper Aug 31 '12 at 20:37

I assume those facts about method, that resizes images:

  • It must return new copy of image. You cannot modify the image itself, because it would break other code that has reference to this image.
  • It doesn't need access to the internal data of Image class. Image classes usually need to offer public access to those data (or copy of).
  • Image resizing is complex and requires many different parameters. Maybe even extensibility points for different resizing algorithms. Passing everything would result in big method signature.

Based on those facts, I would say that there is no reason for Image resizing method be part of Image class itself. Implementing it as static helper method of class would be best.

share|improve this answer
    
Good assumptions. Not sure why it should be a static method though, I try to avoid them for testability. –  ChocoDeveloper Aug 31 '12 at 20:47

An Image Processing class might be appropriate (or Image Manager, as you called it). Pass your Image to a CreateThumbnail method of the Image Processor, for example, to retrieve a thumbnail image.

One of the reasons I'd suggest this route is that you say you're using a 3rd party image processing library. Taking the resizing functionality out of the Image class itself might make it easier for you to isolate any platform specific or 3rd party code. So if you can use your base Image class across all platforms/apps, you then dont have to pollute it with platform or library specific code. That can all be located in the Image Processor.

share|improve this answer
    
Good point. Most people here didn't understand I'm already delegating the most complicated part to a 3rd party library, maybe I wasn't clear enough. –  ChocoDeveloper Aug 31 '12 at 20:32

Basically as Doc Brown already said:

Create a getAsThumbnail() method for the image class, but this method should actually just delegate the work to some ImageUtils class. So it would look something like this:

 class Image{
   // ...
   public Thumbnail getAsThumbnail{
     return ImageUtils.convertToThumbnail(this);
   }
   // ...
 }

And

 class ImageUtils{
   // ...
   public static Thumbnail convertToThumbnail(Image i){
     // ...
   }
   // ...
 }

This will allow easier-to-look-at code. Compare the following:

Image i = ...
someComponent.setThumbnail(i.getAsThumbnail());

Or

Image i = ...
Thumbnail t = ImageUtils.convertToThumbnail(i);
someComponent.setThumbnail(t); 

If the latter one seems good for you, you can also stick to just creating this helper method somewhere.

share|improve this answer

Short answer:

My recomendation is adding this methods the image class:

public Image getResizedVersion(int width, int height);
public Image getResizedVersion(double percentage);

The Image object is still inmutable, these methods return a new image.

share|improve this answer

I think in the "Image Domain", you just have the Image object which is immutable and monadic. You ask the image for a resized version and it returns a resized version of itself. Then you can decide whether you want to get rid of the original or keep both.

Now the thumbnail, avatar, etc versions of the Image is another Domain entirely, which can ask the Image domain for different versions of a certain image to give to a user. Usually this domain is also not so huge or generic, so you can probably keep this in application logic.

In a small scale app, I would resize the images at read time. For example I could have a apache rewrite rule that delegates to php a script if the image 'http://my.site.com/images/thumbnails/image1.png', where the file will be retrieved using the name image1.png and resized and stored in 'thumbnails/image1.png'. Then on the next request to this same image, apache will serve the image directly without running the php script. Your question of findAllImagesWithoutThumbnails is automatically answered by apache, unless you need to do statistics?

In a large scale app, I would send the all new images to a background job, which takes care of generating the different versions of the image and saves it in appropriate places. I wouldn't bother creating a whole domain or class as this domain is very unlikely to grow into a terrible mess of spaghetti and bad sauce.

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.