[Code review] ZipExplorer plugin

Hi, In order to understand better the code I have reviewed the ZipExplorer plugin. Here's my code review: * There are 2 unused fields that should be removed: capacity and log * The getFileName() method can be improved: - first parameter variable must not be modified (the path one in this case). There should always be a local variable created. - I don't think XWiki URI manipulation should be done in this class at all! I'd recommend creating a XWikiURI class in charge of performing all actions on XWiki URI: creation, getting the different part, etc. In this manner is XWiki decides to change the URI format, it'll only be done in one place. This would also remove completely the need for getFileName() from ZipExplorerPlugin. * There are no javadocs... The minimum would be to have a class javadoc explaining what the plugin is doing and putting an example of how to use it. * readFromInputStream() is a generic method to read an input stream in memory. Note that this method should be moved in some generic place as it would be useful for other portion of the code and is completely generic. Even better this method is already implemented in commons-io [http://jakarta.apache.org/commons/io/api-release/org/apache/commons/io/File Utils.html#readFileToByteArray(java.io.File)]. Also note that it's a dangerous method as it's limited by the size of the memory and the size of of a byte[] array in Java... If the zip is large it's going to fail... It would be better to implement it with streaming instead, if possible. * getFileTreeList(). By reading this method's name I don't understand what it does. I also don't understand what is the difference with getFileList() (I haven't analyzed the code). * Exception handling is very very poor :-) For example: } catch (XWikiException e) { e.printStackTrace(); } catch (IOException e) { e.printStackTrace(); } * The unit tests seem to be lacking. In any case they're hard to read. I haven't fully understood them yet... I believe they shouldn't test XWiki's core but only the plugin. Let me know what you think and I'll create some jira issues to fix it. Thanks -Vincent ___________________________________________________________________________ D�couvrez une nouvelle fa�on d'obtenir des r�ponses � toutes vos questions ! Profitez des connaissances, des opinions et des exp�riences des internautes sur Yahoo! Questions/R�ponses http://fr.answers.yahoo.com