+1 to all of these comments.. On Mar 12, 2011, at 10:57 AM, Andy Schwartz <andy.g.schwa...@gmail.com> wrote:
> Yuan - > > The documentation/contract isn't especially clear: > > * This method completes the PPR request and writes a <noop/> to the > response. > * it also handles script and redirect > > I find the first statement is misleading. In the event that a script > or redirect is specified, this method won't generate a <noop/> > response, right? So seems like the doc should be more along the lines > of "completes a ppr response in one of three ways...". > > However, wouldn't it be clearer to break this up into three methods, maybe: > > - completeNoopResponse() > - completeRedirectResponse() > - completeScriptResponse() > > The "PPR" aspect is implied since these static methods live in > PartialPageUtils, so we don't need to duplicate that information in > the method names. > > If the idea is that these methods are only meant to be used for > responding to ppr requests, we should enforce this by spec'ing that an > IllegalStateException is thrown for non-ppr requests. > > Andy
