You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2010/01/16 13:08:05 UTC
Re: svn commit: r899895 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java
On 16/01/2010, bayard@apache.org <ba...@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
Re: svn commit: r899895 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/Range.java
Posted by Henri Yandell <fl...@gmail.com>.
On Sat, Jan 16, 2010 at 4:08 AM, sebb <se...@gmail.com> wrote:
> On 16/01/2010, bayard@apache.org <ba...@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.
Changed. LANG-481 had indicated toString being happy, but I suspect
that was the original report rather than the later issue.
Hen
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org