<Swing Dev> JDK 9 RFR of JDK-8055059: JDK9b22 public API exposes package private classes
Phil Race
philip.race at oracle.com
Tue Aug 19 16:48:27 UTC 2014
I don't know this package very well but it seems like it was a bit of a
messed up design
from the beginning if it exposed methods which had a vector of opaque
types meant
only for internal use. How would an application ever be able to call
them or subclass
them? Amusingly perhaps the javadoc for both these methods actually mention
UndoPosRef by name.
The defining class, GapContent is never used anywhere in public API.
I imagine this to be one example of the many types exposed in Swing only
because
there was no equivalent of C++ friends and it was needed in another package.
I found that apart from the swing.text package in which it is defined it
is used
only once outside its own package, in the swing.text.html sub-package
So I am fairly sure that really the whole GapContent class is for
internal use only
and the same follows for UndoPosRef, so I'd vote against making it public.
I'd actually remove reference to it from thepublic javadoc and say its
an implementation private type.
-phil.
On 8/19/2014 9:16 AM, Anthony Petrov wrote:
> In theory, I agree. However, these methods didn't declare the type of
> objects in the Vectors since they were introduced, and we didn't see
> any RFEs to expose the types, which means that nobody really uses the
> methods outside JDK.
>
> I believe that the missing notice about internal usage of the first
> method is a mistake, it should have been there from the beginning.
>
> Finally, even if you make UndoPosRef visible, it is still an opaque
> type: there's no way to create instances of it, and no any public
> methods or fields. It doesn't make much sense to simply expose the
> name of the type w/o giving developers any control over it (which, as
> we know, has never been requested by those developers yet).
>
> So I'm still unconvinced whether those internal classes should be
> exposed.
>
> --
> best regards,
> Anthony
>
> On 8/19/2014 7:43 PM, Joe Darcy wrote:
>> Hi Anthony,
>>
>> The methods where the UndoPosRef type is used in the generification are
>> protected methods and UndoPosRef occurs in the argument list; from
>> GapContent:
>>
>> protected Vector<UndoPosRef> getPositionsInRange(Vector<UndoPosRef>
>> v, int offset, int length)
>>
>> protected void updateUndoPositions(Vector<UndoPosRef> positions,
>> int offset, int length)
>>
>> The second method includes the disclaimer
>>
>> "This is meant for internal usage, and is generally not of
>> interest to subclasses."
>>
>> but the first method does not. As a protected method, either of these
>> can be called by subclasses. Therefore, it seems to me the subclasses
>> should be able to know the types of the objects in the vectors being
>> passed into or returned.
>>
>> Thanks,
>>
>> -Joe
>>
>> On 08/19/2014 05:34 AM, Anthony Petrov wrote:
>>> Hi Joe,
>>>
>>> I'm not sure if exposing internal classes is a good idea. There's
>>> absolutely no need for user code to even know about them. If the only
>>> reason we have to make them protected is because they're exposed with
>>> the generification fix, then perhaps we could just revert the
>>> generifications (and add corresponding @SuppressWarning) for these
>>> specific cases?
>>>
>>> --
>>> best regards,
>>> Anthony
>>>
>>> On 8/19/2014 5:33 AM, Joe Darcy wrote:
>>>> Hello,
>>>>
>>>> Please review my proposed changes to address:
>>>>
>>>> JDK-8055059: JDK9b22 public API exposes package private classes
>>>> http://cr.openjdk.java.net/~darcy/8055059.0/
>>>>
>>>> Bug JDK-8055059 that the generification of swing added package-private
>>>> types to the signatures of several protected methods. The solution
>>>> is to
>>>> the make the formerly package-private types also be protected.
>>>>
>>>> Patch below.
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>>
>>>> --- old/src/share/classes/javax/swing/text/GapContent.java 2014-08-18
>>>> 18:27:50.000000000 -0700
>>>> +++ new/src/share/classes/javax/swing/text/GapContent.java 2014-08-18
>>>> 18:27:49.000000000 -0700
>>>> @@ -826,7 +826,7 @@
>>>> * Used to hold a reference to a Mark that is being reset as the
>>>> * result of removing from the content.
>>>> */
>>>> - final class UndoPosRef {
>>>> + protected final class UndoPosRef {
>>>> UndoPosRef(MarkData rec) {
>>>> this.rec = rec;
>>>> this.undoLocation = rec.getOffset();
>>>> @@ -839,7 +839,7 @@
>>>> * @param endOffset end location of inserted string.
>>>> * @param g1 resulting end of gap.
>>>> */
>>>> - protected void resetLocation(int endOffset, int g1) {
>>>> + void resetLocation(int endOffset, int g1) {
>>>> if (undoLocation != endOffset) {
>>>> this.rec.index = undoLocation;
>>>> }
>>>> @@ -849,9 +849,9 @@
>>>> }
>>>>
>>>> /** Previous Offset of rec. */
>>>> - protected int undoLocation;
>>>> + private int undoLocation;
>>>> /** Mark to reset offset. */
>>>> - protected MarkData rec;
>>>> + private MarkData rec;
>>>> } // End of GapContent.UndoPosRef
>>>>
>>>>
>>>> --- old/src/share/classes/javax/swing/text/StringContent.java
>>>> 2014-08-18
>>>> 18:27:50.000000000 -0700
>>>> +++ new/src/share/classes/javax/swing/text/StringContent.java
>>>> 2014-08-18
>>>> 18:27:50.000000000 -0700
>>>> @@ -366,7 +366,7 @@
>>>> * Used to hold a reference to a Position that is being reset
>>>> as the
>>>> * result of removing from the content.
>>>> */
>>>> - final class UndoPosRef {
>>>> + protected final class UndoPosRef {
>>>> UndoPosRef(PosRec rec) {
>>>> this.rec = rec;
>>>> this.undoLocation = rec.offset;
>>>> @@ -376,14 +376,14 @@
>>>> * Resets the location of the Position to the offset when
>>>> the
>>>> * receiver was instantiated.
>>>> */
>>>> - protected void resetLocation() {
>>>> + void resetLocation() {
>>>> rec.offset = undoLocation;
>>>> }
>>>>
>>>> /** Location to reset to when resetLocatino is invoked. */
>>>> - protected int undoLocation;
>>>> + private int undoLocation;
>>>> /** Position to reset offset. */
>>>> - protected PosRec rec;
>>>> + private PosRec rec;
>>>> }
>>>>
>>>> /**
>>>>
>>
More information about the swing-dev
mailing list