I'd also ask that you remove the tabs and replace them with spaces :) -T
On Tue, May 26, 2009 at 5:01 PM, Greg Brown <[email protected]> wrote: > The ByteArraySerializer class is looking better. Few more comments (mostly > style): > > - The class doc isn't quite accurate. I'd suggest something simple like > "Implementation of the {...@link Serializer} interface that reads and writes > a byte array." > > - We try to avoid abbreviations with variable names, with the exception of > common variables like "i" for a counter and "n" for a count: > > "BufferedInputStream bis" -> "BufferedInputStream bufferedInputStream" > "BufferedOutputStream bos" -> "BufferedOutputStream(outputStream)" > > - We name all exception variables "exception": > > "catch(IOException ioe)" -> "catch(IOException exception)" > > Otherwise, looks good. Once your test app is done and working, I'd say go > ahead and check it in. > > > On Tuesday, May 26, 2009, at 11:44AM, "Sandro Martini" > <[email protected]> wrote: >>Hi Greg, >>> Just took a look at ByteArraySerializer.java. Overall, looks good. My >>> comments below. >>Thanks. >> >>Uh, i see many small errors caused by haste ... sorry. >>OK, now i have adapted to buffered in/out Streams. >> >>Then, in the writeObject(), i have not put (in a finally block) the >>code that closes the BufferedOutputStream (you said me last day that >>there shouldn't be there flush/close of streams in Serializers), is is >>right also here ? >> >> >>In attach there is the new source, tell me if there are other problems ... >> >>I've just see that this Serializer doesn't have its test class, so if >>needed tomorrow i could create a simple test case for it (and maybe as >>a JUnit 4 Test case), what do you think ? >> >>For the JIRA issue on this file, tell me if i have to close, or if you >>want to do it after commit. >> >> >>On the Coding standard, what do you think on export your Eclipse >>settings and post the file in Subversion (in trunk root, or in tools, >>or in an eclipse-specific folder, or other ...) ? >>So others could simply import it, and if/when needed reformat sources. >> >>Thanks again, >>Sandro >> >> >
