I've just committed the suggested patch + the improvement suggested by Andrea Del Bene to use an interface. If you have better solution please create a new ticket with a patch.
On Tue, Feb 26, 2013 at 9:38 PM, Martin Grigorov <[email protected]>wrote: > Hi, > > > On Tue, Feb 26, 2013 at 9:25 PM, Nuno Pedro Jacinto > <[email protected]>wrote: > >> It wouldn't be better to add a method that can provide information >> concerning what must be done instead of adding more class/interface >> verification? >> I'm sorry if I'm being a little annoying with this, but every time I look >> inside the code I see instanceof everywhere and the last time that I check, >> this is not the most efficient thing. It would probably be better if you >> try to start thinking on a different approach before the framework starts >> to become heavy. >> > > Thanks for your feedback. > By "most efficient" I guess you mean from user point of view. Because > "instanceof" is one of the most efficient JVM instructions, i.e. it is very > fast. > > Please attach a patch with your solution to any of the tickets. > > > >> >> Cheers >> -----Original Message----- >> From: Martin Grigorov [mailto:[email protected]] >> Sent: 25 February 2013 20:40 >> To: [email protected] >> Subject: Re: Review request for UrlResourceReference >> >> On Mon, Feb 25, 2013 at 8:49 PM, Andrea Del Bene <[email protected] >> >wrote: >> >> > I think that the three issue could be solved also introducing an >> > interface for those resource references that want to directly render >> their URL. >> > Something like: >> > >> > public interface IResourceRefUrlGenerator { >> > public Url getUrl(); >> > } >> > >> > Then, inside ReqyestCycle's method renderUrl we can check if resource >> > reference implements this interface and if so, we can delegate Url >> > generation to it. >> > >> >> This looks better than my approach! >> Thanks! >> >> >> > I've attached my patch at WICKET-4907 >> > >> >> Hi, >> >> >> >> At >> >> https://issues.apache.org/**jira/secure/attachment/** >> >> 12570471/WICKET-4907.patch<https://issues.apache.org/jira/secure/atta >> >> chment/12570471/WICKET-4907.patch>you >> >> may see a patch that fixes https://issues.apache.org/** >> >> jira/browse/WICKET-4942<https://issues.apache.org/jira/browse/WICKET- >> >> 4942> , >> >> https://issues.apache.org/**jira/browse/WICKET-4907<https://issues.ap >> >> ache.org/jira/browse/WICKET-4907>and >> >> >> >> https://issues.apache.org/**jira/browse/WICKET-4903<https://issues.ap >> >> ache.org/jira/browse/WICKET-4903> >> >> >> >> The problem is that I don't feel very proud of the solution. >> >> >> >> UrlResourceReference's (URR) purpose is to make it possible to use a >> >> ResourceReference when all you have is a url (absolute or relative) >> >> >> >> There solved problems are: >> >> 1) UrlRenderer#renderUrl() is used after a IRequestMapper resolves a >> Url. >> >> This breaks the provided url in URR by relativizing it >> >> >> >> 2) Until now URR was handled by BasicResourceReferenceMapper which is >> >> wrapped by ParentFolderPlaceholderProvide**r and leads to prefixes >> >> like "::" >> >> in the generated url >> >> >> >> >> >> And what I don't feel happy with is CalculatedUrl. This is a >> >> specialization of Url which UrlRenderer does not try to render. Just >> >> returns it as it is. >> >> >> >> Do you have better ideas how to tell UrlRenderer to not touch the Url >> >> when it comes from UrlResourceReference ? >> >> >> >> >> > >> >> >> -- >> Martin Grigorov >> jWeekend >> Training, Consulting, Development >> http://jWeekend.com <http://jweekend.com/> >> > > > > -- > Martin Grigorov > jWeekend > Training, Consulting, Development > http://jWeekend.com <http://jweekend.com/> > -- Martin Grigorov jWeekend Training, Consulting, Development http://jWeekend.com <http://jweekend.com/>
