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 have multiple entities like Hotels, Destination Cities etc which can contain images. The way I have my system setup right now is, I think of all the images belonging to this universal set (a table in the DB contains filePaths to all the images).

When I have to add an image to an entity, I see if the entity exists in this universal set of images. If it exists, attach the reference to this image, else create a new image. E.g.:

class ImageEntityHibernateDAO {
        public void addImageToEntity(IContainImage entity, String filePath,
                String title, String altText) {
            ImageEntity image = this.getImage(filePath);

            if (image == null)
                image = new ImageEntity(filePath, title, altText);

            getSession().beginTransaction();
                entity.getImages().add(image);
            getSession().getTransaction().commit();

        }
}

My question is: Earlier I had to write this code for each entity (and each entity would have a Set collection). So, instead of re-writing the same code, I created the following interface:

public interface IContainImage {
    Set<ImageEntity> getImages();
}

Entities which have image collections also implements IContainImage interface. Now, for any entity that needs to support adding Image functionality, all I have to invoke from the DAO looks something like this:

// in DestinationDAO::addImageToDestination {
imageDao.addImageToEntity(destination, imageFileName, imageTitle, imageAltText);

// in HotelDAO::addImageToHotel {
imageDao.addImageToEntity(hotel, imageFileName, imageTitle, imageAltText);

It'd be great help if someone can provide me some critique on this design ? Are there any serious flaws that I'm not seeing right away ?

share|improve this question
    
getImage() returns an Image object if it finds it in the database otherwise returns a null. –  brainydexter Mar 15 '12 at 8:36
    
Thanks for your clarification. –  Emmad Kareem Mar 15 '12 at 8:39
1  
question like this would probably be more appropriate at Code Review.SE –  gnat Apr 28 '12 at 14:22

2 Answers 2

How about a straight Add function on the IContainImage interface. e.g.

public interface IContainImage {
    Set<ImageEntity> getImages();
    void Add(ImageEntity);
}

This would read easier:

try
{ 
  getSession().beginTransaction();
  entity.Add(image);
  getSession().getTransaction().commit();
}
except
{
  getSession().getTransaction().Rollback();
}

I have multiple entities like Hotels, Destination Cities etc which can contain images.
It sounds as if you dealing with a classic many to many table relationship. For this you need to introduce a juntion/joining table. That is a table containing the primary key of your entity table and the primary key of your images table.

share|improve this answer

I could be wrong but from the question I got an impression that an Entity could only have one image attached to it. If it is only image per Entity then why to have this kind of structuring? IMHO, you would be paying a performance penalty to search whether an image already exists in set before adding to it.

Wouldn't a simpler design would have been to have a 1:1 relationship between entities like Hotel, Cities and ImageEntity?

Even if you need a set of images for an Entity like Hotel - it should still be a 1:M relationship between them. A universal set is not going to give you the advantage as the image of an entity can't be reused for other entities.

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.