Re: [JGIT PATCH v2 07/24] Added findWorkTree method to Repository class.

Florian Koeberle [off-list ref] wrote:

Isn't `final File workingDirectoryFile = new File(".")` easier to write?

And if we are going to throw a checked exception because we cannot
find a repository in ".", maybe that should be a new checked
exception, like NoGitRepositoryFoundException?

+	/**
+	 * Checks if a path is a valid git repository. Works similar like the method
+	 * is_git_directory from the original setup.c file.

References to C Git aren't common in jgit source code.  We aren't
a reference implementation, but that hasn't stopped people from
finding out code a sane description of "how things work".  I'd rather
not describe our code in terms of C Git's code; especially what
happens if C Git refactors one day, this reference might no longer
make sense.  Anyone coming back here to improve jgit or use this
method will be confused about what we are doing.

+	 * 
+	 * @param directory
+	 *            the path which should be checked.
+	 * @return true if the path is a valid git repository.
+	 */
+	private static boolean isRepository(File directory) {
+		if (!directory.isDirectory()) {
+			return false;
+		}

We usually omit { and } on simple conditions like this.  Its a coding
pattern we stole from C Git, which stole it from the Linux kernel.

+	private static WorkTree findWorkTree(File directory) throws IOException {
+		File currentDirectory = directory.getAbsoluteFile();
+		while (true) {
+			final File commonGitDirectory = new File(directory,
+					REPOSITORY_DIRECTORY_NAME);
+			if (isRepository(commonGitDirectory)) {
+				return new WorkTree(currentDirectory, new Repository(
+						commonGitDirectory));
+			}
+
+			if (isRepository(currentDirectory)) {
+				return new WorkTree(null, new Repository(currentDirectory));
+			}
+			currentDirectory = currentDirectory.getParentFile();
+			if (currentDirectory == null) {
+				throw new IOException("Can't find git repository");
+			}
+		}

Would this perhaps be a shorter, easier to follow variant of the
same algorithm?

	directory = directory.getAbsoluteFile();
	File workdir = directory;
	while (workdir != null) {
		final File gitdir = new File(workdir, REPOSITORY_DIRECTORY_NAME);
		if (isRepository(gitdir))
			return new WorkTree(workdir, new Repository(gitdir));
		if (isRepository(workdir))
			return new WorkTree(null, new Repository(workdir));
		workdir = workdir.getParentFile();
	}
	throw new NoGitRepositoryFoundException("No repository for " + directory);

I often do write infinite loops with "for (;;) {", but usually its
because the loop condition is based on an iterator-style method
returning null at the end and I want to store the result in a local
variable scoped to only the body of the loop:

	for (;;) {
		final RevCommit next = walk.next();
		if (next == null)
			break;

		// use next somehow
	}

Otherwise I try to avoid infinite loops in my final version
of something.  Though on a recent job interview the interviewer
noticed I tend to start writing code with an infinite loop, then
go back and correct the loop condition before saying "done".  ;-)

-- 
Shawn.