<AWT Dev> [9] Review Request: 8037485 Refactor java.awt.datatransfer to eliminate dependency on AWT

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Fri Jul 25 09:17:50 UTC 2014


Hi, Petr.
The fix looks good.
Please update javadoc for:
/**Returns *an Iterator which traverses a* SortedSet of Strings which are*/
public static Set<String> standardEncodings() {}

On 25.07.2014 13:03, Petr Pchelko wrote:
> Hello, Sergey.
>
>> Why we have a duplicate version of StandardEncodings in AddFlavorTest and DataFlavorUtil?
> To avoid referencing internal DataFlavorUtil.
>
> With best regards. Petr.
>
> On 25 июля 2014 г., at 12:59, Sergey Bylokhov <Sergey.Bylokhov at oracle.com> wrote:
>
>> Hi, Petr.
>> Why we have a duplicate version of StandardEncodings in AddFlavorTest and DataFlavorUtil?
>>
>> On 25.07.2014 12:48, Petr Pchelko wrote:
>>> Hello, Peter.
>>>
>>> Sorry for misunderstanding.
>>> I've updated the review: http://cr.openjdk.java.net/~pchelko/9/8037485/webrev.05/
>>>
>>> With best regards. Petr.
>>>
>>> On 25 июля 2014 г., at 10:41, Peter Levart <peter.levart at gmail.com> wrote:
>>>
>>>> On 07/24/2014 01:22 PM, Petr Pchelko wrote:
>>>>> Thank you for the review.
>>>>> I’ve updated the fix:http://cr.openjdk.java.net/~pchelko/9/8037485/webrev.04/
>>>>>
>>>>> I’ve fixed all suggestions except one:
>>>>>>> Another optimization/simplification is possible. You could merge the DesktopDatatransferServiceHolder and default implementation into one class:
>>>>> No, I could not. The default implementation is a member of desktop module while DataFlavorUtil is a part of data transfer module.
>>>>>
>>>> Hi Petr,
>>>>
>>>> Sorry, I haven't been clear/precise enough. I meant to say that you could merge the DesktopDatatransferServiceHolder class and the "fall-back" empty implementation into one class (instead of having DesktopDatatransferServiceHolder a separate class and "fall-back" implementation being anonymous inner class). You save one class and eliminate some boilerplate:
>>>>
>>>> public class DataFlavorUtil {
>>>>
>>>>     public static DesktopDatatransferService getDesktopService() {
>>>>         return DesktopDatatransferServiceImpl.INSTANCE;
>>>>     }
>>>>
>>>>     private static final class DesktopDatatransferServiceImpl implements DesktopDatatransferService {
>>>>         static final DesktopDatatransferService INSTANCE;
>>>>         static {
>>>>             ServiceLoader<DesktopDatatransferService> loader =
>>>>                 ServiceLoader.load(DesktopDatatransferService.class, null);
>>>>             Iterator<DesktopDatatransferService> iterator = loader.iterator();
>>>>             if (iterator.hasNext()) {
>>>>                 INSTANCE = iterator.next();
>>>>             } else {
>>>>                 INSTANCE = new DesktopDatatransferServiceImpl();
>>>>             }
>>>>         }
>>>>
>>>>                     /**
>>>>                      * System singleton FlavorTable.
>>>>                      * Only used if there is no desktop
>>>>                      * to provide an appropriate FlavorMap.
>>>>                      */
>>>>                     private volatile FlavorMap flavorMap;
>>>>
>>>>                     @Override
>>>>                     public void invokeOnEventThread(Runnable r) {
>>>>                         r.run();
>>>>                     }
>>>>
>>>>                     @Override
>>>>                     public String getDefaultUnicodeEncoding() {
>>>>                         return StandardCharsets.UTF_8.name();
>>>>                     }
>>>>
>>>>                     @Override
>>>>                     public FlavorMap getFlavorMap(Supplier<FlavorMap> supplier) {
>>>>                         FlavorMap map = flavorMap;
>>>>                         if (map == null) {
>>>>                             synchronized (this) {
>>>>                                 map = flavorMap;
>>>>                                 if (map == null) {
>>>>                                     flavorMap = map = supplier.get();
>>>>                                 }
>>>>                             }
>>>>                         }
>>>>                         return map;
>>>>                     }
>>>>
>>>>                     @Override
>>>>                     public boolean isDesktopPresent() {
>>>>                         return false;
>>>>                     }
>>>>
>>>>                     @Override
>>>>                     public LinkedHashSet<DataFlavor> getPlatformMappingsForNative(String nat) {
>>>>                         return new LinkedHashSet<>();
>>>>                     }
>>>>
>>>>                     @Override
>>>>                     public LinkedHashSet<String> getPlatformMappingsForFlavor(DataFlavor df) {
>>>>                         return new LinkedHashSet<>();
>>>>                     }
>>>>
>>>>                     @Override
>>>>                     public void registerTextFlavorProperties(String nat, String charset, String eoln, String terminators) {
>>>>                         // Not needed if desktop module is absent
>>>>                     }
>>>>     }
>>>>
>>>>
>>>>
>>>>
>>>> ...but it is good as is.
>>>>
>>>> Regards, Peter
>>>>
>>
>> -- 
>> Best regards, Sergey.
>>


-- 
Best regards, Sergey.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20140725/3501e6fe/attachment.html>


More information about the awt-dev mailing list