[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: Review Request: JDK-8199514: Refactor binding.When
From:       Kevin Rushforth <kevin.rushforth () oracle ! com>
Date:       2018-03-13 16:53:45
Message-ID: 5AA80219.5030804 () oracle ! com
[Download RAW message or body]



Nir Lisker wrote:
>
>     As I understand it, you have added this as a possible refactoring
>     for BooleanConditionBuilder (but left the original in for
>     comparison), right?
>
>
> Yes. I took Boolean as an example to demonstrate the approach.
>
>     Since this would constitute a public API change, I don't think it
>     should be done as part of this RFE.
>
>
> I'm not sure what that change is. Is it extending the 
> private ConditionBuilder class?

Yes. This becomes part of the public API at that point. Also, it 
generally isn't a good practice for a public class to extend non-public 
classes, so that would need to be considered.

-- Kevin

>
>     It will be a couple days before I can look at the rest.
>
>
> No problem.
>
> -Nir 
>
> On Tue, Mar 13, 2018 at 5:42 PM, Kevin Rushforth 
> <kevin.rushforth@oracle.com <mailto:kevin.rushforth@oracle.com>> wrote:
>
>     I took a quick look and had one comment:
>
>        public class BooleanConditionBuilder2 extends
>     ConditionBuilder<Boolean, BooleanBinding> { ... }
>
>     As I understand it, you have added this as a possible refactoring
>     for BooleanConditionBuilder (but left the original in for
>     comparison), right? Since this would constitute a public API
>     change, I don't think it should be done as part of this RFE.
>     Otherwise, it becomes more than just a behavior-neutral
>     implementation refactoring, and would need to be looked at as an
>     API change, with all that entails.
>
>     It will be a couple days before I can look at the rest.
>
>     -- Kevin
>
>
>
>     Nir Lisker wrote:
>
>         Hi,
>
>         Please review preliminary fix for:
>
>         https://bugs.openjdk.java.net/browse/JDK-8199514
>         <https://bugs.openjdk.java.net/browse/JDK-8199514>
>         http://cr.openjdk.java.net/~nlisker/8199514/webrev.00/
>         <http://cr.openjdk.java.net/%7Enlisker/8199514/webrev.00/>
>
>         Thanks,
>         Nir
>          
>
>
[prev in list] [next in list] [prev in thread] [next in thread]