commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r899895 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java
Date Sat, 16 Jan 2010 12:08:05 GMT
On 16/01/2010, bayard@apache.org <bayard@apache.org> wrote:
> Author: bayard
>  Date: Sat Jan 16 07:58:11 2010
>  New Revision: 899895
>
>  URL: http://svn.apache.org/viewvc?rev=899895&view=rev
>  Log:
>  Adding String lazy caching
>
>  Modified:
>     commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java
>
>  Modified: commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java
>  URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java?rev=899895&r1=899894&r2=899895&view=diff
>  ==============================================================================
>  --- commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java (original)
>  +++ commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java Sat
Jan 16 07:58:11 2010
>  @@ -28,13 +28,33 @@
>   */
>   public final class Range<T> implements Serializable {
>
>  +    /**
>  +     * Required for serialization support.
>  +     *
>  +     * @see java.io.Serializable
>  +     */
>      private static final long serialVersionUID = 1L;
>
>  +    /**
>  +     * The ordering scheme used in this range.
>  +     */
>      private final Comparator<T> comparator;
>  +
>  +    /**
>  +     * The minimum value in this range (inclusive).
>  +     */
>      private final T minimum;
>  +    /**
>  +     * The maximum value in this range (inclusive).
>  +     */
>      private final T maximum;
>
>      /**
>  +     * Cached output toString (class is immutable).
>  +     */
>  +    private transient String toString = null;
>  +
>  +    /**
>       * <p>Constructs a new <code>Range</code> using the specified
>       * element as both the minimum and maximum in this range.</p>
>       * <p>The range uses the natural ordering of the elements to
>  @@ -320,16 +340,18 @@
>       */
>      @Override
>      public String toString() {
>  -        StringBuilder buf = new StringBuilder(32);
>  -        buf.append("Range[");
>  -        buf.append(this.minimum);
>  -        buf.append(',');
>  -        buf.append(this.maximum);
>  -        buf.append(']');
>  -        return buf.toString();
>  +        if (toString == null) {
>  +            StringBuilder buf = new StringBuilder(32);
>  +            buf.append("Range[");
>  +            buf.append(this.minimum);
>  +            buf.append(',');
>  +            buf.append(this.maximum);
>  +            buf.append(']');
>  +            toString = buf.toString();
>  +        }
>  +        return toString;

There's some doubt as to whether fetching the toString class variable
twice is thread-safe.

See long thread in Harmony [1] and [2]

[1] http://harmony.markmail.org/message/ud3c6ngyfjeymdga?q=data+reordering+hashCode
[2] http://jeremymanson.blogspot.com/2008/12/benign-data-races-in-java.html

Also HARMONY-6404.

It would be safer to use a temp. local String and return that.

>      }
>
>  -
>      // Taken from Commons Collections - documentation removed as not a public class
>      private static class ComparableComparator<E extends Comparable<? super E>>
implements Comparator<E>, Serializable {
>
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message