[cp-patches] RFA: Container.java

David Gilbert david.gilbert at object-refinery.com
Tue Jun 27 19:37:57 UTC 2006


Tania Bento wrote:

>Hey,
>
>I have implemented the applyComponentOrientation(ComponentOrientation)
>method.  This patch fixes a failing mauve test
>(gnu/testlet/java/awt/Container/applyComponentOrientation). 
>
>Could someone please review my implementation and approve it so that I
>can commit it.  
>
>Cheers,
>Tania
>
>2006-06-27  Tania Bento  <tbento at redhat.com>
>
>        * java/awt/Container.java
>        (applyComponentOrientation): Implemented method.
>  
>
>------------------------------------------------------------------------
>
>Index: java/awt/Container.java
>===================================================================
>RCS file: /cvsroot/classpath/classpath/java/awt/Container.java,v
>retrieving revision 1.94
>diff -u -r1.94 Container.java
>--- java/awt/Container.java	23 Jun 2006 11:00:09 -0000	1.94
>+++ java/awt/Container.java	27 Jun 2006 18:23:04 -0000
>@@ -1594,7 +1594,16 @@
>   public void applyComponentOrientation (ComponentOrientation orientation)
>   {
>     if (orientation == null)
>-      throw new NullPointerException ();
>+      throw new NullPointerException();
>+
>+    this.setComponentOrientation(orientation);
>+    for (int i = 0; i < this.ncomponents; i++)
>+      {
>+        component[i].setComponentOrientation(orientation);
>+        if (component[i] instanceof Container
>+            && ((Container) component[i]).getComponentCount() > 0)
>+          component[i].applyComponentOrientation(orientation);
>+      }
>   }
> 
>   public void addPropertyChangeListener (PropertyChangeListener listener)
>  
>
Your code looks more or less correct, and if it passes all the tests 
then it's certainly a lot better than what we have now (which is totally 
broken).

It looks like there might be some redundancy in there though.  If one of 
the subcomponents is a container, you'll call setComponentOrientation() 
and then call applyComponentOrientation() as well, which seems 
unnecessary.  I haven't tried it, but could the body of the loop be 
replaced by:

if (component[i] instanceof Container)
  ((Container) component[i]).applyComponentOrientation(orientation); 
else
  component[i].setComponentOrientation(orientation);

That drops the check for the component count, which I think is 
unnecessary also...

Regards,

Dave



More information about the Classpath-patches mailing list