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

["Shawn O. Pearce" <spearce@spearce.org>]

mr.gaffo@gmail.com wrote:
> 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.

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