harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jesse Wilson <jessewil...@google.com>
Subject Idiomatic Java: return-at-method-end in Harmony
Date Mon, 26 Oct 2009 21:34:16 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message