<AWT Dev> [PATCH] Cleanup AWT peer interfaces

Roman Kennke roman.kennke at aicas.com
Mon Dec 29 02:27:46 PST 2008


Hi Artem,

> > Cool! I'm comfortable with this fix now.
> > 
> > We still need second approve vote to push this change in.
> > Artem...? Others? ;)
> 
> Well, I'm fine with the proposed changes in general. Still, some JavaDoc 
> comments looks to be too implementation-specific. For example, most of 
> Container's methods contain a statement like this:
> 
> This is called from {@link ...}, before/after ...
> 
> and they really called from the specified places, however this may not 
> be true in future. I'd remove all these statements from JavaDoc, leaving 
> only behavioral description.

Yeah, you are right. I put these in because I found it very useful while
implementing the peers to see exactly where and when it is called. I
removed those comments and only let the @see references in there. The
updated webrev is here:

http://kennke.org/~roman/docpeers/webrev/

Ok now?

Cheers, Roman

> 
> Thanks,
> 
> Artem
> 
> > 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
> >>
-- 
Dipl.-Inform. (FH) Roman Kennke, Software Engineer, http://kennke.org
aicas Allerton Interworks Computer Automated Systems GmbH
Haid-und-Neu-Straße 18 * D-76131 Karlsruhe * Germany
http://www.aicas.com   * Tel: +49-721-663 968-48
USt-Id: DE216375633, Handelsregister HRB 109481, AG Karlsruhe
Geschäftsführer: Dr. James J. Hunt




More information about the awt-dev mailing list