code review exercise for PCA 2014

gistfile1.java

package com.somecompany.app.process;

import java.awt.image.BufferedImage;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.InputStream;
import java.io.OutputStream;

import javax.imageio.ImageIO;

public class PersonPictureProcess extends APPProcess {

	public static byte[] getImage(String id) throws Exception {
		try {
			byte[] imageData = null;

			String filePath = System.getProperty("app.imageRepository")
					+ getFrameworkContext().getPrincipal().getTenant().getId()
					+ "/" + id + ".png";

			File file = new File(filePath);

			if (file.exists()) {
				BufferedImage image = ImageIO.read(new File(filePath));
				ByteArrayOutputStream baos = new ByteArrayOutputStream();
				ImageIO.write(image, "png", baos);
				imageData = baos.toByteArray();
			}

			return imageData;
		} catch (Exception e) {
			throw e;
		}
	}

	public static void setImage(String id, InputStream inputStream,
			String mimeSubType) throws Exception {
		try {
			String filePath = System.getProperty("app.imageRepository")
					+ getFrameworkContext().getPrincipal().getTenant().getId()
					+ "/" + id + "." + mimeSubType;

			File file = new File(filePath);
			file.getParentFile().mkdirs();
			OutputStream outpuStream = new FileOutputStream(new File(filePath));
			int read = 0;
			byte[] bytes = new byte[1024];
			outpuStream = new FileOutputStream(new File(filePath));
			while ((read = inputStream.read(bytes)) != -1) {
				outpuStream.write(bytes, 0, read);
			}
			outpuStream.flush();
			outpuStream.close();
		} catch (Exception e) {
			throw e;
		}
	}

	public static void deleteImage(String id) throws Exception {
		try {
			String filePath = System.getProperty("app.imageRepository")
					+ getFrameworkContext().getPrincipal().getTenant().getId()
					+ "/" + id + ".png";

			File file = new File(filePath);
			file.delete();
		} catch (Exception e) {
			throw e;
		}
	}
}

Well, there is not much that has not yet been touched on, but here are my thoughts on this. I very much agree with most of the things said, but these are the pieces that I saw.

• First off, there are no tests. I am not necessarily saying that TDD needs to be used (although I think there is a VERY strong case for it and I would highly recommend it) but there is nothing to allow a safe refactoring at all
• Rewrite the methods so that the names do not require a comment. If you can not create class names/variable names/method names that are meaningful and explain what the class is doing, there is probably a problem with the design (too big of class, just bad naming, etc)
• This code could really use being DRY'd up. I think everyone agrees on this (see all 432578 comments above)
• Remove the catch statements and add in finally statements to close the streams properly. If you aren't going to handle exceptions, don't bother catching them
• The only thing of value I have to add here is that the methods are written as static, but there is no privatized constructor. Do we really want the user of this class to be able to instantiate the class, hold it in memory, but only access it statically?