Commons Lang
  1. Commons Lang
  2. LANG-576

Add methods for cloneables to ObjectUtils

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0
    • Component/s: lang.*
    • Labels:
      None

      Description

      Object.clone is declared protected, which makes it impossible to write code like:

      if (obj instanceof Cloneable) {
        Object clone = obj.clone();
        ...
      }
      

      Following two methods will help in such a situation:

          /**
           * Clone an object.
           * 
           * @param <T> the type of the object
           * @param o the object to clone
           * @return the clone if the object implements {@link Cloneable} otherwise <code>null</code>
           * @throws CloneFailedException if the object is cloneable and the clone operation fails
           * @since 3.0
           */
          public static <T> T clone(final T o) {
              if (o instanceof Cloneable) {
                  try {
                      final Method clone = o.getClass().getMethod("clone", (Class[])null);
                      @SuppressWarnings("unchecked")
                      final T result = (T)clone.invoke(o, (Object[])null);
                      return result;
                  } catch (final NoSuchMethodException e) {
                      throw new CloneFailedException("Cloneable type has no clone method", e);
                  } catch (final IllegalAccessException e) {
                      throw new CloneFailedException("Cannot clone Cloneable type", e);
                  } catch (final InvocationTargetException e) {
                      throw new CloneFailedException("Exception cloning Cloneable type", e.getCause());
                  }
              }
      
              return null;
          }
      
          /**
           * Clone an object if possible.
           * 
           * @param <T> the type of the object
           * @param o the object to clone
           * @return the clone if the object implements {@link Cloneable} otherwise the object itself
           * @throws CloneFailedException if the object is cloneable and the clone operation fails
           * @since 3.0
           */
          public static <T> T cloneIfPossible(final T o) {
              final T clone = clone(o);
              return clone == null ? o : clone;
          }
      

      Comments? Note, that the code currently introduces also a new CloneFailedException. Use another existing one?

      Unit tests will be provided also.

      1. Cloneable.diff
        5 kB
        Joerg Schaible
      2. CloneableTest.diff
        4 kB
        Joerg Schaible

        Issue Links

          Activity

          Hide
          Oliver Heger added a comment -

          I would like to see support for cloning in lang, too. LANG-307 suggests a CloneUtils class. CloneUtils also provides a method that tries to invoke clone() through reflection. So I think LANG-307 is a super set of this proposal. Maybe the implementations in these proposals can be merged?

          Show
          Oliver Heger added a comment - I would like to see support for cloning in lang, too. LANG-307 suggests a CloneUtils class. CloneUtils also provides a method that tries to invoke clone() through reflection. So I think LANG-307 is a super set of this proposal. Maybe the implementations in these proposals can be merged?
          Hide
          Henri Yandell added a comment -

          No backwards compat issues, so moving to 3.1. If resolved before 3.0 is released please assign to 3.0.

          Show
          Henri Yandell added a comment - No backwards compat issues, so moving to 3.1. If resolved before 3.0 is released please assign to 3.0.
          Hide
          Joerg Schaible added a comment -

          Unit tests for the new methods.

          Show
          Joerg Schaible added a comment - Unit tests for the new methods.
          Hide
          Joerg Schaible added a comment -

          Looking at the comments of LANG-307 I am with Paul. This clone functionality is simple and basic enough (relies completely on the JDK definition of the Cloenable) to be part of ObjectUtils. The clone functionality in CloneUtils tries to do a lot smarter things. Therefore I'd like to commit this for 3.0 if nobody objects.
          When we add CloneUtils later, we can either delegate from ObjectUtils to CloneUtils.cloneByReflection or vice versa.

          Show
          Joerg Schaible added a comment - Looking at the comments of LANG-307 I am with Paul. This clone functionality is simple and basic enough (relies completely on the JDK definition of the Cloenable) to be part of ObjectUtils. The clone functionality in CloneUtils tries to do a lot smarter things. Therefore I'd like to commit this for 3.0 if nobody objects. When we add CloneUtils later, we can either delegate from ObjectUtils to CloneUtils.cloneByReflection or vice versa.
          Hide
          Joerg Schaible added a comment -

          Include into 3.0

          Show
          Joerg Schaible added a comment - Include into 3.0
          Hide
          Joerg Schaible added a comment -

          $ svn ci -m "Add methods for Cloneables to ObjectUtils (LANG-576)."
          Sending src/main/java/org/apache/commons/lang3/ObjectUtils.java
          Adding src/main/java/org/apache/commons/lang3/exception/CloneFailedException.java
          Sending src/test/java/org/apache/commons/lang3/ObjectUtilsTest.java
          Transmitting file data ...
          Committed revision 908745.

          Show
          Joerg Schaible added a comment - $ svn ci -m "Add methods for Cloneables to ObjectUtils ( LANG-576 )." Sending src/main/java/org/apache/commons/lang3/ObjectUtils.java Adding src/main/java/org/apache/commons/lang3/exception/CloneFailedException.java Sending src/test/java/org/apache/commons/lang3/ObjectUtilsTest.java Transmitting file data ... Committed revision 908745.
          Hide
          Henri Yandell added a comment -

          Not convinced by cloneIfPossible, feels like a very undefined state to get into "I may have a clone". Especially given it is the same as 'instanceof Cloneable'.

          I'd like to remove it - thoughts?

          Show
          Henri Yandell added a comment - Not convinced by cloneIfPossible, feels like a very undefined state to get into "I may have a clone". Especially given it is the same as 'instanceof Cloneable'. I'd like to remove it - thoughts?
          Hide
          Joerg Schaible added a comment -

          It's a convenience method when working with immutable objects in concurrency situations that do not have to be cloned at all.

          Show
          Joerg Schaible added a comment - It's a convenience method when working with immutable objects in concurrency situations that do not have to be cloned at all.
          Hide
          Henri Yandell added a comment -

          How so? Do you have an example?

          Basically I'm wondering when I would clone something, yet not care if it was actually cloned or not.

          Show
          Henri Yandell added a comment - How so? Do you have an example? Basically I'm wondering when I would clone something, yet not care if it was actually cloned or not.
          Hide
          Joerg Schaible added a comment -

          We use this for serives loaded with the service API from META-INF/services. However, some of these services are context sensitive or have state and should not be running concurrently. All thouse will implement Cloneable while the others don't care and can run concurrently. With cloneIfPossible is is no convenient simply to pass the original service instance loaded from the SPI and the caller has not to take care of any returned null values or implemented interfaces.

          Show
          Joerg Schaible added a comment - We use this for serives loaded with the service API from META-INF/services. However, some of these services are context sensitive or have state and should not be running concurrently. All thouse will implement Cloneable while the others don't care and can run concurrently. With cloneIfPossible is is no convenient simply to pass the original service instance loaded from the SPI and the caller has not to take care of any returned null values or implemented interfaces.
          Hide
          Henri Yandell added a comment -

          Thanks. I think it's fair to keep the method, but some more javadoc to explain the intended use case would be good.

          Show
          Henri Yandell added a comment - Thanks. I think it's fair to keep the method, but some more javadoc to explain the intended use case would be good.
          Hide
          Joerg Schaible added a comment -

          Javadoc improved

          commit -m "Improve javadoc" lang/src/main/java/org/apache/commons/lang3/ObjectUtils.java
              Sending        lang/src/main/java/org/apache/commons/lang3/ObjectUtils.java
              Transmitting file data ...
              Committed revision 928453.
          

          I'm no native speaker, feel free to correct

          Show
          Joerg Schaible added a comment - Javadoc improved commit -m "Improve javadoc" lang/src/main/java/org/apache/commons/lang3/ObjectUtils.java Sending lang/src/main/java/org/apache/commons/lang3/ObjectUtils.java Transmitting file data ... Committed revision 928453. I'm no native speaker, feel free to correct
          Hide
          Henri Yandell added a comment -

          Thanks Jörg.

          Show
          Henri Yandell added a comment - Thanks Jörg.

            People

            • Assignee:
              Joerg Schaible
              Reporter:
              Joerg Schaible
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development