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]>
