As Ernst explained to me in another bug report, the Java class static methods

java.lang.Byte.valueOf(byte)
java.lang.Double.valueOf(double)
java.lang.Float.valueOf(float)
java.lang.Integer.valueOf(int)
java.lang.Long.valueOf(long)
java.lang.Short.valueOf(short)

should be preferred to the normal constructor or auto-constructing a class wrapper from the primitive value. These static methods maintain a cache of frequently used values.

See https://docs.oracle.com/javase/7/docs/api/java/lang/Long.html#valueOf(long) for the official description, which says that this method should be preferred to calling the default constructor.

What I did is just reviewing the java sources
and making changes to improve quality, readability
and to provide docs.

Hi Mike,
hope that works for you.
I am not a mercurial specialist,
but willing to learn.

Thank you for reviewing and tolerating stress.


(file #38175)

I took 5 minutes and reviewed very briefly. I can offer some basic code formatting suggestions.

• Please delete all trailing whitespace at the end of every line.
• Watch your indentation, I see at least one place that is indented by an extra space, around line 330 of ClassHelper.java.
• Do not break function arguments on every comma, only break when the line exceeds 80 columns.
• I don't know what these "PMD" and "NOPMD" comments mean.
• Don't comment out code that should just be deleted, that's what hg is for.
• Is it really so bad to keep "else if" after a case that does a return? The "else" at the start of the line makes it clear that the cases are related alternatives, otherwise you have to look into the if block to see whether it returns something or not.

If you want to make changes to your patch, please fold those changes into and refresh the existing patch.

I have no comments on the rest, it's Java that is over my head and too deep to review quickly. I have no idea if the changes you have made make sense, or are safe, or do the same thing that they did before. For example you change getClassPath so that it could throw an exception. Is this safe? I don't know. Someone else may, but I can't help review these types of Java functionality changes.

Hi Mike,
Is there something wrong with the patch, that you do not consider it? But then you should tell me that.
How can we proceed??????

Let me patch the patch:

We have to reinsert method

  public static Object createArray (Object cls, int length)

and add some comments to method

  public static Object createArray (Object cls, int[] length)

The following would do:

 //! Return an array with entry type given by @c cls
  //! with dimensions given by @c dims.
  //! The class may be given directly as a @ref Class object
  //! or as a string via its name.
  //! Used by @file javaArray.m defining @c javaArray only.
  //!
  //! Note that if you invoke <code>javaArray('java.lang.Object', 1)</code>,
  //! this is translated into invocation of
  //! <code>ClassHelper.createArray("java.lang.Object", 1)</code>,
  //! invoking ::createArray(Object, int),
  //! whereas <code>javaArray('java.lang.Object', 1, 1)</code>
  //! this is translated into invocation of
  //! <code>ClassHelper.createArray("java.lang.Object", 1, 1)</code>,
  //! invoking ::createArray(Object, int[]).
  //!
  //! @param cls
  //!    the atomic component class given as a class object or as a string.
  //!    If given as a string, the class is loaded via #loader.
  //! @param dims
  //!    the dimensions which shall be non-negative but less than 256.
  //! @return
  //!    an array with the given component type and dimensions.
  //!    If @c cls is a string, the class is loaded by #loader.
  //! @throws ClassNotFoundException
  //!    if @c cls is a string representing a class name
  //!    and if #loader could not find the according class.
  //! @see ::createArray(Object, int)
  // based on loader but only partially.
  // used by javaArray.m
  public static Object createArray (Object cls, int[] dims)
    throws ClassNotFoundException
  {
    // FIXME: seems as if used in javaArray.m only.
    // Then more appropriate seems split into two methods:
    // one with cls is a Class other one with cls is a String
    // FIXME: if a class is given directly,
    // it is not guaranteed, that this is loaded via #loader
    // So this shall be fixed.
    // Typically, different usages are treated in m file invoking error
    // in the last else branch.
    // This would be appropriate here also.
    Class theClass;
    if (cls instanceof Class)
      {
        theClass = (Class) cls;
      }
    else if (cls instanceof String)
      {
        // may throw ClassNotFoundException
        theClass = Class.forName ((String) cls, true, loader);
      }
    else
      {
        throw new IllegalArgumentException ("invalid class specification " +
                                            cls);
      }

    return Array.newInstance (theClass, dims);
  }


  //! Return a one-dimensional array with entry type given by @c cls
  //! with length by @c length.
  //! The class may be given directly as a @ref Class object
  //! or as a string via its name.
  //! Used by @file javaArray.m defining @c javaArray only.
  //!
  //! @param cls
  //!    the atomic component class given as a class object or as a string.
  //!    If given as a string, the class is loaded via #loader.
  //! @param length
  //!    the length of the array to be created.
  //! @return
  //!    an array with the given component type and length.
  //!    If @c cls is a string, the class is loaded by #loader.
  //! @throws ClassNotFoundException
  //!    if @c cls is a string representing a class name
  //!    and if #loader could not find the according class.
  //! @see ::createArray(Object, int)
  // based on loader but only partially.
  // used by javaArray.m
  // would be superfluous with varargs introduced in java 1.5:
  // the createArray (Object cls, int[] length)
  // could be written createArray (Object cls, int... length)
  public static Object createArray (Object cls, int length)
    throws ClassNotFoundException
  {
    return createArray (cls, new int[] {length});
  }

Hi,
i just endowed part of sources with doxygen comments,
added comments where i see a bug,
simplified implementation (partially replaced recursion by loops),
and declared throwables more carefully.