Re: SpringLayout

[Michael Koch]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am Freitag, 4. Juni 2004 10:27 schrieb Roman Kennke:
> Hi list,
>
> I've implemented javax.swing.SpringLayout and javax.swing.Spring. I
> have not yet signed the FSF copyright agreement, so I cannot get
> this into Classpath immediatly. Nevertheless I post it here for
> review for anybody interested. There's a good chance that this is
> buggy, so I would be glad about testing.
> The patch also contains a little addition to javax.swing.ImageIcon.
> (A ImageIcon(java.awt.Image) constructor).
>
> I've also started to write some Mauve tests for it, the first test
> ist also attached as diff against the Mauve tree.
>
> Suggestions are welcome.

Here are my comments:

diff -rNPu classpath/javax/swing/ImageIcon.java 
classpath-roman/javax/swing/ImageIcon.java
- --- classpath/javax/swing/ImageIcon.java        2004-04-29 
09:00:34.000000000 +0200
+++ classpath-roman/javax/swing/ImageIcon.java  2004-06-03 
15:47:56.000000000 +0200
@@ -70,6 +70,14 @@
         //loadImage(image);
     }
 
+    public ImageIcon(Image image) {

Please put '{' on new line

+        return;

No need for this.

+package javax.swing;
+
+/**
+ * DOCUMENT ME!

Please add dcoumentation from the beginning. Otherwise it will never 
be written ....

+class SimpleSpring extends Spring

You can make this an inner class. Thats IMO better.

+    private int min, pref, max, value;

One line for each variable (probably with documentation) is better.

+      return;

No need.

+    public int getValue() {

Bracket on a newline please.

+      int max1 = s1.getMaximumValue();
+      int max2 = s2.getMaximumValue();
+      return max1 + max2;

Why not "return s1.getMaximumValue() + s2.getMaximumValue();" ?

+                  if ((retVal == null) && (y != null) && (height !=
 null))

Its more readable when each condition is on a new line with the 
connecting operator on front of the line.


Thanks, for your work. I have only checked this for style now. I think 
you get the ideas of the my style hints.


Michael
- -- 
Homepage: http://www.worldforge.org/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFAwJGxWSOgCCdjSDsRAhC6AJ9OJXosBdT2CCyiwzVEmz0C9uyZSgCfZaen
4DKE3D6q61u8OsXLSljotf8=
=WqYn
-----END PGP SIGNATURE-----