[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [cp-patches] Suggesting headers for public methods for javax.swing.T
From: |
Michael Koch |
Subject: |
Re: [cp-patches] Suggesting headers for public methods for javax.swing.Timer |
Date: |
Thu, 24 Feb 2005 22:58:17 +0100 |
User-agent: |
mutt-ng 1.5.8i (Debian) |
On Thu, Feb 24, 2005 at 10:37:56PM +0100, Meskauskas Audrius wrote:
> This class is undocumented. I suggest some headers at least for for public
> methods and also tried to apply several Michael's recommendations on
> formatting.
> The code itself is not altered.
Some comments from me.
>@@ -34,45 +34,47 @@
> this exception to your version of the library, but you are not
> obligated to do so. If you do not wish to do so, delete this
> exception statement from your version. */
>-
>-
> package javax.swing;
>
>-import java.awt.event.ActionEvent;
>-import java.awt.event.ActionListener;
> import java.io.Serializable;
> import java.util.EventListener;
>
>+import java.awt.event.ActionEvent;
>+import java.awt.event.ActionListener;
> import javax.swing.event.EventListenerList;
We sort them imports alphabeticaly and group them by there first package
part name, e.g. java, javax, gnu, etc.
>- private static final long serialVersionUID = -1116180831621385484L;
>+ private final static long serialVersionUID = -1116180831621385484L;
We use the modidier order specified by JLS which is "static final"
>- /** DOCUMENT ME! */
> private Object queueLock = new Object();
Even private stuff should be documented. Just removing the signal to
document it is of no help. Some explaining words are always good,
>
>- /** DOCUMENT ME! */
> private Waker waker;
>
>- private Runnable drainer = new Runnable()
>+ private Runnable drainer =
>+ new Runnable()
The new should be on the same line.
> {
> public void run()
> {
>@@ -80,103 +82,105 @@
> }
> };
>
>- /**
>- * DOCUMENT ME!
>- */
> private void queueEvent()
> {
> synchronized (queueLock)
>- {
>- queue++;
>- if (queue == 1)
>- SwingUtilities.invokeLater(drainer);
>- }
>+ {
>+ queue++;
>+ if (queue == 1)
>+ SwingUtilities.invokeLater(drainer);
>+ }
This should be indented as it was. We indent by two spaces after
keywords, exceptions are class, interface and method bodies only.
>- if (isCoalesce())
>- {
>- if (queue > 0)
>- fireActionPerformed();
>- }
>- else
>- {
>- while (queue > 0)
>- {
>- fireActionPerformed();
>- queue--;
>- }
>- }
>- queue = 0;
>- }
>+ {
>+ if (isCoalesce())
>+ {
>+ if (queue > 0)
>+ fireActionPerformed();
>+ }
>+ else
>+ {
>+ while (queue > 0)
>+ {
>+ fireActionPerformed();
>+ queue--;
>+ }
>+ }
>+ queue = 0;
>+ }
> }
Same here.
>
>+ /**
>+ * If true, the timer prints a message to <code>System.out</code>
>+ * when firing each event.
>+ */
> static boolean logTimers;
>
>- /** DOCUMENT ME! */
>+ /**
>+ * True if the timer coalesces events.
>+ */
> boolean coalesce = true;
>
>- /** DOCUMENT ME! */
>+ /**
>+ * True if the timer is firing repetetive events.
>+ */
> boolean repeats = true;
>
>- /** DOCUMENT ME! */
>+ /**
>+ * True if the timer is currently active, firing events as scheduled.
>+ */
> boolean running;
>
>- /** DOCUMENT ME! */
> int ticks;
>
>- /** DOCUMENT ME! */
>+ /**
>+ * The delay between subsequent repetetive events.
>+ */
> int delay;
>
>- /** DOCUMENT ME! */
>- int initialDelay;
>-
> /**
>- * DOCUMENT ME!
>+ * The initial delay before the first event.
> */
>- private class Waker extends Thread
>+ int initialDelay;
>+
>+ private class Waker
>+ extends Thread
> {
>- /**
>- * DOCUMENT ME!
>- */
> public void run()
> {
> running = true;
> try
> {
>- sleep(initialDelay);
>-
>- queueEvent();
>-
>- while (running)
>- {
>- try
>- {
>- sleep(delay);
>- }
>- catch (InterruptedException e)
>- {
>- return;
>- }
>- queueEvent();
>-
>- if (logTimers)
>- System.out.println("javax.swing.Timer -> clocktick");
>-
>- if (! repeats)
>- break;
>- }
>- running = false;
>+ sleep(initialDelay);
>+
>+ queueEvent();
>+
>+ while (running)
>+ {
>+ try
>+ {
>+ sleep(delay);
>+ }
>+ catch (InterruptedException e)
>+ {
>+ return;
>+ }
>+ queueEvent();
>+
>+ if (logTimers)
>+ System.out.println("javax.swing.Timer -> clocktick");
>+
>+ if (!repeats)
>+ break;
>+ }
>+ running = false;
Same here.
> }
> catch (Exception e)
> {
>-// System.out.println("swing.Timer::" + e);
>+ // System.out.println("swing.Timer::" + e);
This debug output should be totally removed and replaced by
"// Ignored." or somthing like this.
> }
> }
> }
>@@ -191,16 +195,19 @@
> public Timer(int d, ActionListener listener)
> {
> delay = d;
>- initialDelay = d;
>+ initialDelay = d;
>
> if (listener != null)
> addActionListener(listener);
> }
>
> /**
>- * DOCUMENT ME!
>+ * Sets whether the Timer coalesces multiple pending event firings.
>+ * If the coalescing is enabled, the multiple events that have not been
>+ * fired on time are replaced by the single event. The events may not
>+ * be fired on time if the application is busy.
> *
>- * @param c DOCUMENT ME!
>+ * @param c true (default) to enable the event coalescing, false otherwise
true and false should be enclosed in <code>...</code> to make them look
a little different in the final javadocs.
> */
> public void setCoalesce(boolean c)
> {
>@@ -208,9 +215,12 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Checks if the Timer coalesces multiple pending event firings.
>+ * If the coalescing is enabled, the multiple events that have not been
>+ * fired on time are replaced by the single event. The events may not
>+ * be fired on time if the application is busy.
> *
>- * @return DOCUMENT ME!
>+ * @return true if the coalescing is enabled, false otherwise
> */
> public boolean isCoalesce()
> {
>@@ -218,9 +228,9 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Add the action listener
> *
>- * @param listener DOCUMENT ME!
>+ * @param listener the action listener to add
> */
> public void addActionListener(ActionListener listener)
> {
>@@ -228,9 +238,9 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Remove the action listener.
> *
>- * @param listener DOCUMENT ME!
>+ * @param listener the action listener to remove
> */
> public void removeActionListener(ActionListener listener)
> {
>@@ -238,12 +248,13 @@
> }
>
> /**
>- * DOCUMENT ME!
>- *
>- * @param listenerType DOCUMENT ME!
>+ * Get the event listeners of the given type that are listening for the
>+ * events, fired by this timer.
> *
>- * @return DOCUMENT ME!
>+ * @param listenerType the listener type (for example, ActionListener.class)
> *
>+ * @return the array of event listeners that are listening for the events,
>+ * fired by this timer
> * @since 1.3
> */
> public EventListener[] getListeners(Class listenerType)
>@@ -252,9 +263,10 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Get the array of action listeners.
> *
>- * @return DOCUMENT ME!
>+ * @return the array of action listeners that are listening for the events,
>+ * fired by this timer
> *
> * @since 1.4
> */
>@@ -264,20 +276,20 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Fire the given action event to the action listeners.
> *
>- * @param event DOCUMENT ME!
>+ * @param event the event to fire
> */
> protected void fireActionPerformed(ActionEvent event)
> {
>- ActionListener[] listeners = getActionListeners();
>+ ActionListener listeners[] = getActionListeners();
Please use Java array syntax and not C/C++ array syntax. The [] has to
be after the type and not after the identifier.
> for (int i = 0; i < listeners.length; i++)
>- listeners[i].actionPerformed(event);
>+ listeners [ i ].actionPerformed(event);
We dont write the whitespaces, "listeners[i]" was okay.
> }
>
> /**
>- * DOCUMENT ME!
>+ * Fire the action event, named "Timer".
> */
> void fireActionPerformed()
> {
>@@ -285,9 +297,10 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Set the timer logging state. If it is set to true, the timer prints
>+ * a message to <code>System.out</code> when firing each action event.
> *
>- * @param lt DOCUMENT ME!
>+ * @param lt true if logging is enabled, false (default value) otherwise
> */
> public static void setLogTimers(boolean lt)
> {
>@@ -295,9 +308,10 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Return the logging state.
> *
>- * @return DOCUMENT ME!
>+ * @return true if the timer is printing a message to
><code>System.out</code>
>+ * when firing each action event
> */
> public static boolean getLogTimers()
> {
>@@ -305,9 +319,11 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Set the delay between firing the subsequent events.
>+ * This parameter does not change the value of the initial delay before
>+ * firing the first event.
> *
>- * @param d DOCUMENT ME!
>+ * @param d The time gap between the subsequent events, in milliseconds
> */
> public void setDelay(int d)
> {
>@@ -315,9 +331,9 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Get the delay between firing the subsequent events.
> *
>- * @return DOCUMENT ME!
>+ * @return The delay between subsequent events, in milliseconds
> */
> public int getDelay()
> {
>@@ -325,9 +341,12 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Set the intial delay before firing the first event since calling
>+ * the <code>start()</code> method. If the initial delay has not been
>+ * set, it is assumed having the same value as the delay between the
>+ * subsequent events.
> *
>- * @param i DOCUMENT ME!
>+ * @param i the initial delay, in milliseconds
> */
> public void setInitialDelay(int i)
> {
>@@ -335,9 +354,11 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Get the intial delay before firing the first event since calling
>+ * the <code>start()</code> method. If the initial delay has not been
>+ * set, returns the same value as <code>getDelay()</code>.
> *
>- * @return DOCUMENT ME!
>+ * @return the initial delay before firing the first action event.
> */
> public int getInitialDelay()
> {
>@@ -345,9 +366,10 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Enable firing the repetetive events.
> *
>- * @param r DOCUMENT ME!
>+ * @param r true (default value) to fire repetetive events. False to fire
>+ * only one event after the initial delay
> */
> public void setRepeats(boolean r)
> {
>@@ -355,9 +377,10 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Check is this timer fires repetetive events.
> *
>- * @return DOCUMENT ME!
>+ * @return true if the timer fires repetetive events, false if it fires
>+ * only one event after the initial delay
> */
> public boolean isRepeats()
> {
>@@ -365,9 +388,10 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Get the timer state.
> *
>- * @return DOCUMENT ME!
>+ * @return true if the timer has been started and is firing the action
>+ * events as scheduled. False if the timer is inactive.
> */
> public boolean isRunning()
> {
>@@ -375,7 +399,7 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Start firing the action events.
> */
> public void start()
> {
>@@ -386,7 +410,8 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Cancel all pending tasks and fire the first event after the initial
>+ * delay.
> */
> public void restart()
> {
>@@ -395,7 +420,7 @@
> }
>
> /**
>- * DOCUMENT ME!
>+ * Stop firing the action events.
> */
> public void stop()
> {
>@@ -403,8 +428,8 @@
> if (waker != null)
> waker.interrupt();
> synchronized (queueLock)
>- {
>- queue = 0;
>- }
>+ {
>+ queue = 0;
>+ }
The old indention of { and } were correct but the "queue = 0;" seems to
miss two spaces of indention.
Thanks for your work. I dont want to be pedantic. I just want to get a
make our coding style the same in whole classpath over time.
The javadoc comments are really good.
Michael