From: Shawn O. Pearce <spearce@spearce.org>
Date: Tue, 11 Nov 2008 18:26:26 GMT
Subject: Re: [EGIT PATCH 5/7] Add a method to get refs by object Id
Message-ID: <20081111182626.GP2932@spearce.org>
In-Reply-To: <1226095664-13759-6-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -952,6 +955,37 @@ public Ref peel(final Ref ref) {
>  	}
>  
>  	/**
> +	 * @return a map with all objects referenced by a peeled ref.
> +	 */
> +	public Map<AnyObjectId, Set<Ref>> getAllRefsByPeeledObjectId() {
> +		Map<String, Ref> allRefs = getAllRefs();
> +		Map<AnyObjectId, Set<Ref>> ret = new HashMap<AnyObjectId, Set<Ref>>(allRefs.size());
> +		for (Ref ref : allRefs.values()) {
> +			if (ref == null)
> +				continue;

How did we get a null Ref inside the allRefs collection?

> +			if (!ref.isPeeled()) {
> +				ref = peel(ref);
> +				allRefs.put(ref.getOrigName(), ref);

Hmm.  Mutating a HashMap while you are traversing it with an Iterator
is *not* a good idea.  Its a ConcurrentModificationException waiting
to happen.

-- 
Shawn.