<AWT Dev> [PATCH] Cleanup AWT peer interfaces

Andrei V. Dmitriev Andrei.Dmitriev at Sun.COM
Mon Dec 15 06:31:13 PST 2008


Hi Roman,

Cool! I'm comfortable with this fix now.

We still need second approve vote to push this change in.
Artem...? Others? ;)

Thanks,
   Andrei

Roman Kennke wrote:
> Hi Andrei,
> 
> Finally I came around to fix the suggested stuff. See comments inline.
> 
>> 1) src/share/classes/java/awt/peer/CheckboxMenuItemPeer.java
>>
>> +import java.awt.CheckboxMenuItem;
>> if that's really needed? I thought that {@link ...} doesn't require such 
>> stuff.
> 
> I don't know. If you fully qualify the stuff in the {@link } it's not
> needed. Should I fully-qualify things everywhere or let the import in
> place? I don't care really.
> 
> 
>> 2) src/share/classes/java/awt/peer/ContainerPeer.java
>> There is a typo in the second word:
>> -     * Indicates availabiltity of restacking operation in this container.
>> +     * Indicates availability of restacking operation in this container.
> 
> Fixed.
> 
>> 3) Common thing.
>> <code>true</code> have a shorter synonym since JDK 5: {@code true}
> 
> Fixed.
> 
>> 4) src/share/classes/java/awt/peer/RobotPeer.java
>> I noticed that you prefer not to leave "public" modifier in an 
>> interface. But here you are leaving all of them.
> 
> Fixed.
> 
>> 5) src/share/classes/java/awt/peer/RobotPeer.java
>> public int getNumberOfButtons(); has left w/o any comments.
> 
> Fixed. I'm not sure if I got the sematics right.
> 
>> 6) src/share/classes/java/awt/peer/ComponentPeer.java
>> +    /**
>> +     * Called by {@link EventQueue#coalescePaintEvent} to let the component
>> +     * peer coalesce paint events.
>> +     *
>> +     * @param e the paint event to consider to coalesce
>> +     *
>> +     * @see EventQueue#coalescePaintEvent
>> +     */
>> +    void coalescePaintEvent(PaintEvent e);
>>
>> I'd say it's not "to let .... coalesce paint events", but "to coalesce 
>> paint events".
> 
> Fixed.
> 
>> 7) at the same class.
>> You wrote:
>> +    // TODO: Maybe change this to force Graphics2D, since many things will
>> +    // break with plain Graphics nowadays.
>> +    Graphics getGraphics();
>>
>> Do you know a scenario to show what's exactly might be broken. We 
>> probably need to introduce another peer for that, right?
> 
> I already explained my reason in earlier mails. You think this is
> reasonable?
> 
> 
>> 8) Similar to 7):
>> +    // TODO: Maybe make that return a BufferedImage, because some stuff 
>> will
>> +    // break if a different kind of image is returned.
>> +    Image createImage(int width, int height);
> 
> Dito.
> 
> This time I did the work on top of the -awt workspace and created a
> webrev for your reviewing pleasure:
> 
> http://kennke.org/~roman/docpeers/webrev/
> 
> Thanks for having a look at it again,
> 
> Roman
> 



More information about the awt-dev mailing list