Re: [Review request] 8145143: Promote Image.impl_getUrl() to public API as Image.getUrl()
The patch looks good to me as it is. Question: Will the change to Optional makes it at odd with the existing public constructors? Or API cleanliness though I do see the beauty of using Optional here. - Chien On 12/10/15, 2:16 PM, David Grieve wrote: I believe the url might be used in the CSS engine for caching images loaded from .css files. If the css image cache is made public (I don't know if that is planned, but it should be considered), then this API will matter. On 12/10/15 4:39 PM, Kevin Rushforth wrote: That's an interesting question. We haven't used Optional for such cases except the return value of Dialog showAndWait. Wrapping the URL in an Optional is more self-documenting, but perhaps less convenient if you know that it is non-null (and basically the same if you don't know, since this doesn't seem like something you would do anything with other than asking "is it valid"). Another interesting question: how useful is this API? It was on the list of impl_* method that Jonathan and I felt would be worth making public API out of, but I don't know how prevalent its use is? -- Kevin Tom Schindl wrote: As i see it the url is optional so wouldn't make sense to return Optional to make this more explicit? Tom Von meinem iPhone gesendet Am 10.12.2015 um 21:54 schrieb Jonathan Giles : Hi Chien, Kevin, Please can you review the following JBS issue (and attached patch): https://bugs.openjdk.java.net/browse/JDK-8145143 As the subject line states, this issue proposes to promote the Image.impl_getUrl() method to Image.getUrl(), so that it is public API. -- -- Jonathan
Re: [Review request] 8145143: Promote Image.impl_getUrl() to public API as Image.getUrl()
I believe the url might be used in the CSS engine for caching images loaded from .css files. If the css image cache is made public (I don't know if that is planned, but it should be considered), then this API will matter. On 12/10/15 4:39 PM, Kevin Rushforth wrote: That's an interesting question. We haven't used Optional for such cases except the return value of Dialog showAndWait. Wrapping the URL in an Optional is more self-documenting, but perhaps less convenient if you know that it is non-null (and basically the same if you don't know, since this doesn't seem like something you would do anything with other than asking "is it valid"). Another interesting question: how useful is this API? It was on the list of impl_* method that Jonathan and I felt would be worth making public API out of, but I don't know how prevalent its use is? -- Kevin Tom Schindl wrote: As i see it the url is optional so wouldn't make sense to return Optional to make this more explicit? Tom Von meinem iPhone gesendet Am 10.12.2015 um 21:54 schrieb Jonathan Giles : Hi Chien, Kevin, Please can you review the following JBS issue (and attached patch): https://bugs.openjdk.java.net/browse/JDK-8145143 As the subject line states, this issue proposes to promote the Image.impl_getUrl() method to Image.getUrl(), so that it is public API. -- -- Jonathan
Re: [Review request] 8145143: Promote Image.impl_getUrl() to public API as Image.getUrl()
Since this is a read-only get method of an immutable property, I guess it doesn't matter much. We can make it an Optional if that's what most folks want. -- Kevin Jonathan Giles wrote: I tend to be a fan of Optionalbut it isn't something we have used much of in JavaFX (I might be the only user of it so far in the Dialog API). From my perspective then, I can +1 on the Optional, but we'll see what others say. -- Jonathan On 11/12/15 10:27 AM, Tom Schindl wrote: As i see it the url is optional so wouldn't make sense to return Optional to make this more explicit? Tom Von meinem iPhone gesendet Am 10.12.2015 um 21:54 schrieb Jonathan Giles : Hi Chien, Kevin, Please can you review the following JBS issue (and attached patch): https://bugs.openjdk.java.net/browse/JDK-8145143 As the subject line states, this issue proposes to promote the Image.impl_getUrl() method to Image.getUrl(), so that it is public API. -- -- Jonathan
Re: [Review request] 8145143: Promote Image.impl_getUrl() to public API as Image.getUrl()
That's an interesting question. We haven't used Optional for such cases except the return value of Dialog showAndWait. Wrapping the URL in an Optional is more self-documenting, but perhaps less convenient if you know that it is non-null (and basically the same if you don't know, since this doesn't seem like something you would do anything with other than asking "is it valid"). Another interesting question: how useful is this API? It was on the list of impl_* method that Jonathan and I felt would be worth making public API out of, but I don't know how prevalent its use is? -- Kevin Tom Schindl wrote: As i see it the url is optional so wouldn't make sense to return Optional to make this more explicit? Tom Von meinem iPhone gesendet Am 10.12.2015 um 21:54 schrieb Jonathan Giles : Hi Chien, Kevin, Please can you review the following JBS issue (and attached patch): https://bugs.openjdk.java.net/browse/JDK-8145143 As the subject line states, this issue proposes to promote the Image.impl_getUrl() method to Image.getUrl(), so that it is public API. -- -- Jonathan
Re: [Review request] 8145143: Promote Image.impl_getUrl() to public API as Image.getUrl()
I tend to be a fan of Optionalbut it isn't something we have used much of in JavaFX (I might be the only user of it so far in the Dialog API). From my perspective then, I can +1 on the Optional, but we'll see what others say. -- Jonathan On 11/12/15 10:27 AM, Tom Schindl wrote: As i see it the url is optional so wouldn't make sense to return Optional to make this more explicit? Tom Von meinem iPhone gesendet Am 10.12.2015 um 21:54 schrieb Jonathan Giles : Hi Chien, Kevin, Please can you review the following JBS issue (and attached patch): https://bugs.openjdk.java.net/browse/JDK-8145143 As the subject line states, this issue proposes to promote the Image.impl_getUrl() method to Image.getUrl(), so that it is public API. -- -- Jonathan
Re: [Review request] 8145143: Promote Image.impl_getUrl() to public API as Image.getUrl()
As i see it the url is optional so wouldn't make sense to return Optional to make this more explicit? Tom Von meinem iPhone gesendet > Am 10.12.2015 um 21:54 schrieb Jonathan Giles : > > Hi Chien, Kevin, > > Please can you review the following JBS issue (and attached patch): > > https://bugs.openjdk.java.net/browse/JDK-8145143 > > As the subject line states, this issue proposes to promote the > Image.impl_getUrl() method to Image.getUrl(), so that it is public API. > > -- > > > -- Jonathan >
[Review request] 8145143: Promote Image.impl_getUrl() to public API as Image.getUrl()
Hi Chien, Kevin, Please can you review the following JBS issue (and attached patch): https://bugs.openjdk.java.net/browse/JDK-8145143 As the subject line states, this issue proposes to promote the Image.impl_getUrl() method to Image.getUrl(), so that it is public API. -- -- Jonathan
