myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Blake Sullivan <>
Subject Re: [Trinidad] ChangeManagerWrapper API
Date Thu, 10 Mar 2011 20:25:02 GMT
The names can't be the same because if we implemented the Wrapper 
interface, the new method would have to be public, however if someone 
had subclassed the protected method, keeping it protected, that change 
would break their code because you can't subclass a method and reduce 
its visibility.

-- Blake Sullivan

On 3/10/11 12:20 PM, Scott O'Bryan wrote:
> I suppose I don't mind the currently implementation but I'm not sure I 
> understand Point #2.  Adding an interface does not change binary 
> backward compatibility does it?  Likewise, having a public 
> getWrapped() method wouldn't have any conflicts if the interface was 
> applied later.
> From a "pretty code" POV, I like the idea of wrappers being 
> implemented in a consistent fashion.  Takes away a lot of confusion.  
> In the end though, Trinidad has a lot of wrappers which are already 
> implemented in different ways.  What's one more going to hurt.. :D
> Scott
> On 03/10/2011 11:37 AM, Andy Schwartz wrote:
>> Thanks for the review Pavitra.  I too was debating whether to follow
>> the FacesWrapper approach.  In the end I leaned towards keeping the
>> get*Wrapper method protected, since I couldn't think of any non-hacky
>> reason for exposing this publicly.
>> On Wed, Mar 9, 2011 at 6:13 PM, Blake Sullivan
>> <>  wrote:
>>> 1) Is because there is currently no good reason for developers to 
>>> burrow
>>> into the ChangeManager implementations
>> Right.
>>> 2) Is because if we did come up with a good reason to implement
>>> FacesWrapper<T>, we need to be able to do so in a backwards compatible
>>> manner
>> Exactly.  My initial implementation contained a protected getWrapped()
>> method, though I ended up picking a different name for this protected
>> hook since I wanted to leave open the possibility of implementing
>> FacesWrapper (and exposing a public getWrapped() method) in the future
>> should the need arise.
>> Andy

View raw message