Also, did you compare the performance of using an array versus a HashSet?
On 21 April 2014 19:44, Matt Sicker <[email protected]> wrote: > Here's what I'm changing that contains method to: > > private static boolean contains(final Marker parent, final > Marker... localParents) { > //noinspection ForLoopReplaceableByForEach > for (int i = 0, localParentsLength = localParents.length; i < > localParentsLength; i++) { > final Marker marker = localParents[i]; > if (marker == parent) { > return true; > } > } > return false; > } > > > > On 21 April 2014 19:42, Matt Sicker <[email protected]> wrote: > >> Which brings up another issue with markers. In MarkerManager, we have a >> volatile array of Markers. Here's the message from IntelliJ: >> >> Reports array fields which are declared as *volatile*. Such fields may >> be confusing, as accessing the array itself follows the rules for >> *volatile* fields, but accessing the array's contents does not. If such >> volatile access is needed to array contents, the JDK5.0 >> *java.util.concurrent.atomic* classes should be used instead. >> >> Is this relevant here? >> >> >> On 21 April 2014 19:37, Matt Sicker <[email protected]> wrote: >> >>> 1) that would be my bad. I usually replace those with foreach loops >>> where possible. It's usually good to comment in those cases. I'll revert >>> that and comment. >>> >>> 2) that makes more sense than the exception >>> >>> >>> On 21 April 2014 18:46, Bruce Brouwer <[email protected]> wrote: >>> >>>> I saw that some small changes were being made to the Markers. I had a >>>> few thoughts regarding them: >>>> >>>> 1) Use of array iterator instead of indexed for loop. >>>> for (Marker marker : localParents) >>>> instead of >>>> for (int i = 0; i < localParents.length; i++) >>>> >>>> When I was doing my performance benchmarks, I was finding the latter to >>>> be faster. I'm guessing this is simply because a new Iterable object needs >>>> to be created to iterate over the array. >>>> >>>> For most methods, such as add, remove, this was not a big deal. But for >>>> the isInstanceOf and checkParent methods, we want those to be as fast as >>>> possible. >>>> >>>> 2) isInstanceOf(String markerName) >>>> Instead of throwing an IllegalArgumentException when a marker of name >>>> markerName doesn't exist, why don't we simply return false? I don't want an >>>> IllegalArgumentException to happen because I'm testing a markerName. >>>> >>>> >>>> >>>> -- >>>> >>>> Bruce Brouwer >>>> >>> >>> >>> >>> -- >>> Matt Sicker <[email protected]> >>> >> >> >> >> -- >> Matt Sicker <[email protected]> >> > > > > -- > Matt Sicker <[email protected]> > -- Matt Sicker <[email protected]>
