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

[Robin Rosenberg <robin.rosenberg.lists@dewire.com>]

fredagen den 23 maj 2008 20.12.31 skrev Florian Köberle:
> >> +	 * 
> >> +	 * @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.
> 
> I used this style once too, but was convinced that it dangerous to do so.
As Shawn said, usually. Sometimes we slip too.  But it is unnecessary and
anything unnecessary usually makes code harder to read. It is not a big
deal to me. 

> The following looks correct at the first look, but it's not:
> 
> 	if (a)
> 		if (b)
> 			something;
> 	else
> 		something;
> 
> this can't happen if you use { and }:
> 	if (a) {
> 		if (b) {
> 			something0();
> 		}
> 	} else {
> 		something1();
> 	}

This isn't the same case, because here we have nested statements, I do think braces should be used here. Java shouldn't even allow "ambigous" syntax like this (first case).

> Also it is better extenable:
> 
> if (a) {
> 	something0();
> }
> 
> if (a) {
> 	something0():
> +	something1();
> }

Which is why I think is not that big a deal.  The main reason I may leave braces on a non-nested simple statement is becuase I had a debug statement or something like that there, and I might well want to have one again temporarily. Toggling braces on and off (even with eclipse where it requires two-four keypresses) are annoying.

-- robin