jsr372-experts@javaserverfaces-spec-public.java.net

[jsr372-experts] Re: [jsr372-experts mirror] Add constructor taking wrapped to all FacesWrapper classes

From: Neil Griffin <neil.griffin_at_portletfaces.org>
Date: Mon, 25 Jul 2016 16:47:56 -0400

+1 but it would be interesting to know why the reduction of boilerplate wasn't part of the wrapper classes from the beginning. The absence makes it seem intentional.

> On Jul 23, 2016, at 5:54 PM, Josh Juneau <juneau001_at_gmail.com> wrote:
>
> +1, I agree that consistency makes sense.
>
> Josh Juneau
> juneau001_at_gmail.com
> http://jj-blogger.blogspot.com
> https://www.apress.com/index.php/author/author/view/id/1866
>
>
> On Sat, Jul 23, 2016 at 4:42 PM, Bauke Scholtz <balusc_at_gmail.com> wrote:
> Hi,
>
> Check all implementing classes of FacesWrapper: https://docs.oracle.com/javaee/7/api/javax/faces/FacesWrapper.html When extending such a FacesWrapper class like ResourceWrapper you always end up with below minimally required boilerplate:
>
>
> public class MyResource extends ResourceWrapper {
> private Resource wrapped;
>
> public MyResource(Resource wrapped) {
> this.wrapped = wrapped;
> }
>
> @Override
> public Resource getWrapped() {
> return wrapped;
> }
> }
>
>
> Compare with a.o. http://docs.oracle.com/javaee/7/api/javax/servlet/http/HttpServletRequestWrapper.html It has an explicit constructor taking wrapped instance and already a default getWrapped() implementation returning exactly that.
>
>
> public class MyRequest extends HttpServletRequestWrapper {
> public MyRequest(HttpServletRequest wrapped) {
> super(wrapped);
> }
> }
>
>
> You just have to push wrapped to super() and there's already a default implementation for getWrapped(). This also forces developers to use getWrapped() instead of the local variable. Because, when developers use the local variable, the decorator chain may break when the wrapper instance is in turn wrapped by another wrapper instance (as can be seen in a.o. ResponseWriterWrapper when using MyFaces as its PartialResponseWriter extends from it but internally doesn't correctly use getWrapped()).
>
> I propose improving all FacesWrapper classes to add such a constructor (along with the default one) so that boilerplate can be reduced and robustness can be improved.
>
> Thoughts?
>
> Cheers, B
>
>
>
>