http://codereview.appspot.com/217091/diff/1/3 File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java (right):
http://codereview.appspot.com/217091/diff/1/3#newcode84 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:84: public static final class ConcatUri { On 2010/02/23 22:52:14, zhoresh wrote:
add class javadoc that explain snippet
Done. http://codereview.appspot.com/217091/diff/1/3#newcode103 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:103: public static final class ValidatedUri extends ProxyUriBase { Yikes, good catch. Added to this CL. http://codereview.appspot.com/217091/diff/1/4 File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java (right): http://codereview.appspot.com/217091/diff/1/4#newcode73 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:73: // Above params are common for all generated Uris. On 2010/02/23 22:52:14, zhoresh wrote:
Since it is common, it sounds like a conversion that can be extracted
to a
utility class or a base class for all UriManagers
By this I mean that these params are common for all ConcatUris, so I pass the base Uri into makeConcatUri (this sort of is the utility method). That said, at least one other UriManager (ProxyUriManager) is quite similar... perhaps a possibility to factor out common logic exists there. http://codereview.appspot.com/217091/diff/1/4#newcode85 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:85: String version = versions != null ? versions.get(i++) : null; On 2010/02/23 22:52:14, zhoresh wrote:
Maybe iterator on versions instead of index
Done. http://codereview.appspot.com/217091/show
