Re: [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files.

From: Shawn O. Pearce <spearce@spearce.org>
Date: Sep 21, 2009, 19:40 UTC
Message-ID: <20090921194035.GM14660@spearce.org>
In-Reply-To: <1253062116-13830-3-git-send-email-mr.gaffo@gmail.com>

mr.gaffo@gmail.com wrote:

Show 5 quoted lines

> diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
> +	public void testListLocalPacksWhenThereIsAPack() throws Exception {
> +		createTestDir();
> +		File packsDir = new File(testDir, "pack");
> +		packsDir.mkdirs();

Why not allow the ObjectDirectory code to create the directory before copying the pack into it?

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
> +	/**
> +	 * The list of Packs THIS repo contains

Don't you mean the list of packs this object database contains? An object database may not be a git repository. Though yes, the common case is that it is a repository.

> +	 * @return List<PackFile> of package names contained in this repo. 
> +	 * 		   Should be an empty list if there are none.
> +	 */
> +	public abstract List<PackFile> listLocalPacks();

I think you should define this to be an unmodifiable list, not just any list. Its sad that the Java type system didn't support this idea back when they added the new collections APIs.

Show 5 quoted lines

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
> +	@Override
> +	public List<PackFile> listLocalPacks() {
> +		tryAgain1();
> +		return new ArrayList<PackFile>(Arrays.asList(packList.get().packs));

Instead of copying, why not return an unmodifiableList wrapped around the array? PackList will never modify its internal array.

-- 
Shawn.