jakarta-bcel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dale E Martin <dmar...@cliftonlabs.com>
Subject how about this kind of a patch?
Date Fri, 15 Feb 2002 23:18:22 GMT
OK, I've joined the mailing list.  I know that I'm completely unknown among
this group, but I do a fair amount of Java and C++ programming and I tend
to refactor things as I work as a habit.  Here is a small patch that
unifies the constructors of ConstantPool:

[start patch]
--- ConstantPool.java-	Fri Feb 15 17:40:02 2002
+++ ConstantPool.java	Fri Feb 15 17:57:50 2002
@@ -34,16 +34,17 @@
    */
   ConstantPool(DataInputStream file) throws IOException, ClassFormatError
   {
-    byte tag;
-
-    constant_pool_count = file.readUnsignedShort();
-    constant_pool       = new Constant[constant_pool_count];
+    this( readConstantsFromStream( file ) );
+  }
 
-    /* constant_pool[0] is unused by the compiler and may be used freely
+  private static Constant [] readConstantsFromStream( DataInputStream file )
+    throws IOException, ClassFormatError {
+    Constant [] retval = new Constant[file.readUnsignedShort()];
+    /* retval[0] is unused by the compiler and may be used freely
      * by the implementation.
      */
-    for(int i=1; i < constant_pool_count; i++) {
-      constant_pool[i] = Constant.readConstant(file);
+    for(int i=1; i < retval.length; i++) {
+      retval[i] = Constant.readConstant(file);
 	  
       /* Quote from the JVM specification:
        * "All eight byte constants take up two spots in the constant pool.
@@ -52,10 +53,11 @@
        * 
        * Thus we have to increment the index counter.
        */
-      tag = constant_pool[i].getTag();
-      if((tag == Constants.CONSTANT_Double) || (tag == Constants.CONSTANT_Long))
+      if((retval[i].getTag() == Constants.CONSTANT_Double) || 
+	 (retval[i].getTag() == Constants.CONSTANT_Long))
 	i++;
     }
+    return retval;
   }
 
   /**
@@ -269,7 +271,8 @@
   }    
 
   /**
-   * @param constant Constant to set
+   * @param index Must be in the range of constant_pool_count.
+   * @param constant Constant to set.
    */
   public void setConstant(int index, Constant constant) {
     constant_pool[index] = constant;
[end patach]

Are these types of changes desired?  I don't want to be stepping on
anyone's toes, and I realize everyone's sense of style varies.  One could
argue that this is a style patch, although in reviewing the code I was
initially not 100% sure of the relationship of "constant_pool" and
"constant_pool_count".  After applying this patch it seems more obvious to
me that the next patch could be to rewrite "getLength()" to return
"constant_pool.length" which would get rid of some data replication.

Other questions - there are lots of warnings when compiling with jikes in
pedantic mode.  Most of them are stylistic - I can provide examples if
you're interested, and/or patches.  Also, is there a regression test that I
can run - junit tests or something like that?

Thanks,
	Dale
-- 
Dale E. Martin, Clifton Labs, Inc.
Senior Computer Engineer
dmartin@cliftonlabs.com
http://www.cliftonlabs.com
pgp key available

--
To unsubscribe, e-mail:   <mailto:bcel-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:bcel-dev-help@jakarta.apache.org>


Mime
View raw message