Author: nbelaevski
Date: 2009-03-25 15:29:00 -0400 (Wed, 25 Mar 2009)
New Revision: 13200
Modified:
trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/FileManagerTest.java
trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ImageLoader.java
Log:
Code review results committed
Modified:
trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/FileManagerTest.java
===================================================================
---
trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/FileManagerTest.java 2009-03-25
19:07:03 UTC (rev 13199)
+++
trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/FileManagerTest.java 2009-03-25
19:29:00 UTC (rev 13200)
@@ -4,6 +4,8 @@
import org.richfaces.realworld.manager.FileManager;
+//TODO nick - test should be moved to src\main\test
+//TODO nick - test doesn't pass
public class FileManagerTest extends TestCase {
protected void setUp() throws Exception {
Modified:
trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ImageLoader.java
===================================================================
---
trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ImageLoader.java 2009-03-25
19:07:03 UTC (rev 13199)
+++
trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ImageLoader.java 2009-03-25
19:29:00 UTC (rev 13200)
@@ -50,6 +50,8 @@
@In("#{messages['file_processing_error']}")
private String FILE_PROCESSING_ERROR;
+ //TODO nick - synchronized would make this a bottleneck, especially considering that
this method
+ // consumes plenty of CPU time
public synchronized void paintImage(OutputStream out, Object data) throws IOException
{
if (null == data) {
return;
@@ -59,17 +61,23 @@
FileInputStream fileInputStream = new FileInputStream(imageResource);
BufferedInputStream bufferedInputStream = new
BufferedInputStream(fileInputStream);
InputStream paintData = bufferedInputStream;
+ //TODO nick - paintData is never null
if (null == paintData) {
Events.instance().raiseEvent(Constants.ADD_ERROR_EVENT, new
Exception(FILE_PROCESSING_ERROR));
return;
}
try {
- BufferedImage images = ImageIO.read(paintData);
+ //TODO nick - pass-through writing will save us large amount of CPU time
and images quality
+ BufferedImage images = ImageIO.read(paintData);
ImageIO.write(images, Constants.JPEG, out);
} catch (Exception e) {
- Events.instance().raiseEvent(Constants.ADD_ERROR_EVENT, new
Exception(FILE_PROCESSING_ERROR));
- return;
+ //TODO nick - any particular reason to catch not to take the whole method
body into try/catch?
+ //TODO nick - how useful is new Exception(...)? why not use just String?
+ Events.instance().raiseEvent(Constants.ADD_ERROR_EVENT, new
Exception(FILE_PROCESSING_ERROR));
+ //TODO nick - that's not necessary at all
+ return;
} finally {
+ //TODO nick - calling bufferedInputStream.close() is enough
fileInputStream.close();
bufferedInputStream.close();
paintData.close();
@@ -77,6 +85,7 @@
}
}
+ //TODO nick - this is copy-pasted 99% from paintImage()
public synchronized void paintImageFromFile(OutputStream out, Object data) throws
IOException {
if (null == data) {
return;
Show replies by date