On Tue, Mar 2, 2010 at 1:36 AM, <[email protected]> wrote: > some simple nits.. > > otherwise looks good. > > > > http://codereview.appspot.com/224074/diff/5/1004 > File > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java > (right): > > http://codereview.appspot.com/224074/diff/5/1004#newcode62 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:62: > > Any reason to use transients? I don't think this class will ever be > serialized... >
Not that I'm aware of... I thought it odd members were marked transient before. Removed. > > http://codereview.appspot.com/224074/diff/5/1004#newcode108 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:108: > if (jsonVar.matches("^\\w*$")) { > consider creating a static pattern or use a better word matching pattern > StringUtils.isAlphaNum(jsonVar)|| jsonVar.contains("_") may be equiv... > Good idea; changed to static Pattern for the moment. > > http://codereview.appspot.com/224074/diff/5/1004#newcode121 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:121: > > null check for cos perhaps... This does look weird, but AFAICT there's no possible way for cos to be null since all conditional branches above it either set its value or return. > > > http://codereview.appspot.com/224074/show >
