1
edit
Changes
→Project Phases
===Phase 7: Code Review 3 (Done) ===
* Send patch to NexJ for review
===Phase 8: Apply Changes per Code Review 3(Done)===*JSONWriter.java:**Can a StringBuilder be used instead of a StringBuffer? Maybe the same change can be done in TextMarshaller as well? The member field can still be called m_buffer. Using StringBuilder instead of StringBuffer avoids the synchronization overhead.**writeString: remove the unnecessary conversion to a character array; the length can be accessed via String.length().*JSONMarshaller.java**Can you fill in the comment for parameter “writer” on method “marshal” of interface Marshaller? It is currently marked with TODO.**You can use the string id err.rpc.mshType instead of err.rpc.json.mshType**Can you define the Timestamp marshaller first, then the java.util.Date marshaller? Vassiliy wanted the default framework type’s marshaller first, followed by its synonyms.*JSONParser.java**Putting parts of the error message in the error arguments Object array that is passed to fail() isn’t localizable. The error message should be part of the value of the string id. String ids are stored in the *.strings files. For string ids used by Java code, the .strings files are in core/src/nexj/core/meta/sys. You need to ensure that there is an entry in this file for each id that you use. These are then translated into French (and possibly other languages) by people who specialize in translating software interfaces. The only things passed in the error arguments Object array should be data.**There should be a blank line before “break” and “return” statements.*JSONUnmarshaller.java**For the castTo* methods, can you rename them to get* and have them take a String sKey argument, which is the key to lookup in m_valueMap? This will more closely follow what Vassiliy intended when he said “Rather than directly type casting values retrieved from json map, you can have unmsh accessor methods for the common types, which can also do error checking, so that one does not get a generic type cast error and wonder what went wrong and where.”**parseObject(): I don’t think you need separate branches of code for handling the first key in the iterator and subsequent keys. Also, for iteration, prefer for() to while().*JSONUnmarshallerException.java**The overload with (String sExpected, Class actual) isn’t localizable. Remove this overload and use the (String sErrCode, Object[] argArray) constructor. In JSONUnmarshaller get* methods, pass an Object[] of length two for the argArray. Ensure that both are strings.**Please add a string for err.rpc.json.unmsh.CastException to en.strings in core/src/nexj/core/meta/sys. The various parts of the string id should all start with lowercase letters, e.g. CastException à castException. Use a meaningful name instead of “CastException”—CastException is what it is, not what the error is.*JSONTests.java**testSerializeException doesn’t need to initialize the marshaller and unmarshaller: they are initialized already by setUp(), which is run before each test*() method.**please remove the parts of tests that are duplicated. For example, in testServerObjects() you test the serialization of a Symbol called “sym”, a PrivilegeSet of “480000”, and a PCodeFunction with code 0, 1, 2. But these have already been tested by the “testSerializeRequestCheckJSON” which uses the m_request object initialized by the superclass.**Use Primitive.createInteger, Primitive.createLong, etc. instead of new Integer(…), new Long(…), etc. where possible.===Phase 9: Code Review 4 ===* Send patch to NexJ for review
==Links==
'''JSON''' <br/>
http://www.json.org <br/>
