I like literate threads from Jesse just because they recall understanding and participation feelings. :-)
On Tue, Oct 27, 2009 at 12:06 PM, Tim Ellison <[email protected]> wrote: > As I general rule (which is, of course, made to be broken as you sit in > front of the editor where it suddenly doesn't make sense), I'll try to: > > - keep the methods short, factoring out to private helpers, so that > each method is understandable, and details pushed down to collections of > other simple methods, > > - perform the argument verification and precondition checks first, > returning early if they do not hold (so in this case, I agree with you > readConfiguration() checks that return early), > > - try to have the method return at the end, rather than having multiple > return points throughout (of course, there is always that case where you > need to return from inside a loop, which to me always feels like a jail > break<g>) > > I suspect that we are all not too far from adopting the same style of > programming. I'd hate for us to get into the practice of rewriting code > that works just to fix the style, when there are some real interesting > problems to solve. And as a rule breaker, yes, I do go in and fix the > style of working code! > > Regards, > Tim > > On 26/Oct/2009 21:34, Jesse Wilson wrote: >> Harmony Team, >> >> Some of the Java code in Harmony suffers from being written in a non-Java >> style. In particular, our Java code often attempts to limit the number of >> exit points from a method. This approach is common in C/C++ programs because >> of the need to manually collect garbage. But the approach has no advantages >> in Java. I dislike it because it requires the reader to carry more in their >> mental stack. >> >> Consider LogManager.readConfiguration(), the bulk of which lives inside an >> if() block: >> >> public void readConfiguration() throws IOException { >> >> // check config class >> >> String configClassName = System >> >> .getProperty("java.util.logging.config.class"); >> >> if (null == configClassName >> >> || null == getInstanceByClass(configClassName)) { >> >> // if config class failed, check config file >> >> String configFile = System >> >> .getProperty("java.util.logging.config.file"); >> >> >> if (null == configFile) { >> >> // if cannot find configFile, use default logging.properties >> >> configFile = new StringBuilder().append( >> >> >> System.getProperty("java.home")).append(File.separator) >> >> .append("lib").append(File.separator).append( >> >> "logging.properties").toString(); >> >> } >> >> >> InputStream input = null; >> >> try { >> >> input = new BufferedInputStream(new >> FileInputStream(configFile)); >> >> readConfiguration(input); >> >> } finally { >> >> if (input != null) { >> >> try { >> >> input.close(); >> >> } catch (Exception e) {// ignore >> >> } >> >> } >> >> } >> >> } >> >> } >> >> >> By using an eager return, the code needs fewer layers of indentation and >> thus feels simpler (at least to the colleagues I've asked): >> >> public void readConfiguration() throws IOException { >> >> // check config class >> >> String configClassName = System >> >> .getProperty("java.util.logging.config.class"); >> >> if (null != configClassName >> >> && null != getInstanceByClass(configClassName)) { >> >> return; >> >> } >> >> >> // if config class failed, check config file >> >> String configFile = System >> >> .getProperty("java.util.logging.config.file"); >> >> >> if (null == configFile) { >> >> // if cannot find configFile, use default logging.properties >> >> configFile = new StringBuilder().append( >> >> System.getProperty("java.home")).append(File.separator) >> >> .append("lib").append(File.separator).append( >> >> "logging.properties").toString(); >> >> } >> >> >> InputStream input = null; >> >> try { >> >> input = new BufferedInputStream(new >> FileInputStream(configFile)); >> >> readConfiguration(input); >> >> } finally { >> >> if (input != null) { >> >> try { >> >> input.close(); >> >> } catch (Exception e) {// ignore >> >> } >> >> } >> >> } >> >> } >> >> >> In patches I'll be submitting, I'll use the second form, which is idiomatic >> Java. I may also submit patches that convert the first style to the second, >> but usually only when there's other useful cleanup to accompany it. If you'd >> rather I not, please let me know and we'll arm-wrestle. >> >> >> Cheers, >> Jesse >> > -- With best regards / с наилучшими пожеланиями, Alexei Fedotov / Алексей Федотов, http://www.telecom-express.ru/ http://harmony.apache.org/ http://www.expressaas.com/ http://openmeetings.googlecode.com/
