harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pavel Pervov <pmcfi...@gmail.com>
Subject Re: Idiomatic Java: return-at-method-end in Harmony
Date Mon, 26 Oct 2009 22:39:55 GMT
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 <jessewilson@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
>

Mime
View raw message