I'd support Jesse here. This way, layers-on-layers-on-layers make code
even more error prone once a change should be introduced in it.
So, I'm all +1 for decreasing indentation level.
Pavel.
On Tue, Oct 27, 2009 at 12:34 AM, Jesse Wilson <jessewil...@google.com> 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
>
