https://issues.apache.org/bugzilla/show_bug.cgi?id=48018
Josh Micich <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |NEEDINFO --- Comment #5 from Josh Micich <[email protected]> 2009-10-21 14:44:36 UTC --- Rather than just making arbitrary changes to the code, it would be nice to have some additional justification - junits are usually a good start. Can you give details as to how the current code causes errors? I understand very little about the code that you are changing, but there seems to be a few outstanding issues in your patch that either need fixing or explaining. ListLevel.java Problems with the order of field serialisation. the current order is: <init>() { .., cbGrpprlChpx, cbGrpprlPapx, .., grpprlPapx, grpprlChpx, .. } toByteArray() { .., cbGrpprlChpx, cbGrpprlPapx, .., grpprlChpx, grpprlPapx, .. } but you suggest: <init>() { .., cbGrpprlPapx, cbGrpprlChpx, .., grpprlPapx, grpprlChpx, .. } toByteArray() { .., cbGrpprlPapx, cbGrpprlChpx, .., grpprlPapx, grpprlChpx, .. } So the patch does the exact opposite of what you describe in (comment 0) which suggests the correct code should be 'Ch' before 'Pa'. Note - the ordering in the current code is internally inconsistent at lines 106-109 (the only place that currently has 'Pa' before 'Ch'). Perhaps this is the one place you wanted to change. I am pretty sure that the proposed change at line 122 makes no difference: - _numberText[x] = (char)LittleEndian.getShort(buf, offset); + _numberText[x] = (char)LittleEndian.getUShort(buf, offset); ListTables.java You've made a new field _listArr which seems to store the same data as _listMap. Around line 106 should there be an extra line added, similar to what you have done at line 62? _listMap.put(new Integer(lsid), lst); + _listArr.add(lst); It would also be good to add a comment explaining why you're forcing the ListData objects to be serialised in the same order they were read in. Lastly, can you make your changes with respect to the latest code from trunk? Your original change seems to be from prior to svn r805492 . Try composing the patch using "svn diff" or "git diff". Patch/diff files are easier to manage. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
