Skip to content

Commit

Permalink
Make EmptyLineCheck Thread safe
Browse files Browse the repository at this point in the history
Just better to make it defensively thread safe. Not sure how the
checkstyle API is calling it.
  • Loading branch information
JimDeanSpivey committed Feb 1, 2016
1 parent 644e6a3 commit 04a23f3
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,13 @@
* your code and make it more cohesive and readable. The bottom line is
* that every method should look solid and do just <b>one thing</b>.
*
* This class is <b>not</b> thread safe. It relies on building a list of line
* ranges by visiting each method definition and each anonymous inner type.
* This class is thread safe. It relies on building a list of line ranges by
* visiting each method definition and each anonymous inner type. It stores
* these references in a non-static thread local.
*
* @author Krzysztof Krason (Krzysztof.Krason@gmail.com)
* @author Yegor Bugayenko (yegor@tpc2.com)
* @author Jimmy Spivey (JimDeanSpivey@gmail.com)
* @version $Id$
*/
public final class EmptyLinesCheck extends Check {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,23 @@ public final class LineRanges {
/**
* ArrayList of line ranges.
*/
private final transient Collection<LineRange> lines =
new ArrayList<LineRange>(20);
private final transient LineRanges.LocalCollection lines =
new LineRanges.LocalCollection();

/**
* Adds a line range to the internal collection.
* @param line The line range to add to the collection
*/
public void add(final LineRange line) {
this.lines.add(line);
this.lines.collection().add(line);
}

/**
* Expses the iterator of the internal collection.
* @return Iterator pointing to the internal collections elements.
*/
public Iterator<LineRange> iterate() {
return this.lines.iterator();
return this.lines.collection().iterator();
}

/**
Expand All @@ -73,7 +73,8 @@ public Iterator<LineRange> iterate() {
* @return True if the proposed line number is within any line range.
*/
public boolean inRange(final int line) {
return !this.lines.isEmpty() && FluentIterable.from(this.lines)
return !this.lines.collection().isEmpty()
&& FluentIterable.from(this.lines.collection())
.anyMatch(new LineRanges.LineWithAny(line));
}

Expand All @@ -88,7 +89,7 @@ public LineRanges within(final LineRanges ranges) {
final Iterator<LineRange> iterator = ranges.iterate();
while (iterator.hasNext()) {
final LineRange next = iterator.next();
for (final LineRange line : this.lines) {
for (final LineRange line : this.lines.collection()) {
if (next.within(line)) {
result.add(line);
}
Expand All @@ -101,7 +102,7 @@ public LineRanges within(final LineRanges ranges) {
* Clears the internal collection.
*/
public void clear() {
this.lines.clear();
this.lines.collection().clear();
}

/**
Expand All @@ -128,4 +129,25 @@ public boolean apply(final LineRange range) {
return range != null && range.within(this.proposed);
}
}

/**
* Thread safety via ThreadLocal.
*/
private static final class LocalCollection
extends ThreadLocal<Collection<LineRange>> {

/**
* Internal Collection.
*/
private final transient Collection<LineRange> ranges =
new ArrayList<LineRange>(20);

/**
* Get the collection specific to the current thread only.
* @return The collection for this thread.
*/
public Collection<LineRange> collection() {
return this.ranges;
}
}
}

0 comments on commit 04a23f3

Please sign in to comment.