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

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

From: arjan tijms <arjan.tijms_at_gmail.com>
Date: Tue, 26 Jul 2016 09:20:42 +0200

+1 great idea!

On Tue, Jul 26, 2016 at 12:56 AM, Leonardo Uribe <leonardo.uribe_at_irian.at>
wrote:

> +1
>
> 2016-07-25 15:47 GMT-05:00 Neil Griffin <neil.griffin_at_portletfaces.org>:
>
>> +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
>> >
>> >
>> >
>> >
>>
>>
>