You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Jesse Wilson <je...@google.com> on 2009/10/26 22:34:16 UTC

Idiomatic Java: return-at-method-end in Harmony

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

Re: Idiomatic Java: return-at-method-end in Harmony

Posted by Pavel Pervov <pm...@gmail.com>.
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 <je...@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
>

Re: Idiomatic Java: return-at-method-end in Harmony

Posted by Mark Hindess <ma...@googlemail.com>.
In message <c3...@mail.gmail.com>,
Alexey Petrenko writes:
>
> I agree that the code readability is good. No doubt here.
> 
> Unfortunately, rewriting the code which works fine but does not follow
> some our internal vision can easily offend the original authors and
> provoke holy wars.

Would any original authors really be offended or surprised that code
they "donated" to a larger project is refactored to achieve consistent
style across a project?

We are not overrun with current contributors at the moment so I think
making the code readable and thus lowering the barrier to contribution
has to be a good thing for the project.  I think that most original
authors would feel that it is a more worthwhile goal to ensure a
useful future for the code they have donated than to "preserve" it.

> I'm following the same policy while accepting or rejecting the code
> into the project: if the code works fine, do not have visible
> performance or quality issues, but formatted in the way I do not
> like much then I better accept it with minimal modifications to keep
> the author happy. And let him work on the other real problems but
> try to convince me to return back his original code or explain my
> modifications.

I follow a similar policy.  Contributions don't have to be perfect.
They have to be useful in furthering the goals of the project *and*
they have to have a reasonable chance of being maintained - either by
the contributors or by the community.

You seem to imply that original code should be preserved.  I disagree.
It should be developed to serve the projects goals to the utmost extent.
Isn't that why it was donated?  If the original authors feel that the
project goals would be best served by not making particular changes then
we are very open to their input.

> However, I feel ok to rewrite the code I do not like completely if I'm
> making some important modifications :)

I'm not sure this helps because:

a) you will never achieve consistency (okay we probably wont anyway
   but this approach guarantees it),

b) if anyone felt strongly enough to have a "holy war"[0] about style
   then they'll probably be the ones doing the same over the
   definition of "important modificiation".

Regards,
 Mark.

[0] I'm not sure such contributors exist but perhaps they do and they're
    just waiting for the right moment to speak up. ;-)



Re: Idiomatic Java: return-at-method-end in Harmony

Posted by Alexey Petrenko <al...@gmail.com>.
I agree that the code readability is good. No doubt here.

Unfortunately, rewriting the code which works fine but does not follow
some our internal vision can easily offend the original authors and
provoke holy wars.

I'm following the same policy while accepting or rejecting the code
into the project: if the code works fine, do not have visible
performance or quality issues, but formatted in the way I do not like
much then I better accept it with minimal modifications to keep the
author happy. And let him work on the other real problems but try to
convince me to return back his original code or explain my
modifications.

However, I feel ok to rewrite the code I do not like completely if I'm
making some important modifications :)

Thanks.

Alexey

2009/10/27 Tim Ellison <t....@gmail.com>:
> 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
>>
>

Re: Idiomatic Java: return-at-method-end in Harmony

Posted by Alexei Fedotov <al...@gmail.com>.
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 <t....@gmail.com> 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/

Re: Idiomatic Java: return-at-method-end in Harmony

Posted by Tim Ellison <t....@gmail.com>.
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
> 

Re: Idiomatic Java: return-at-method-end in Harmony

Posted by Nathan Beyer <nd...@apache.org>.
On Mon, Oct 26, 2009 at 6:15 PM, Jesse Wilson <je...@google.com> wrote:
> On Mon, Oct 26, 2009 at 3:44 PM, Mark Hindess
> <ma...@googlemail.com>wrote:
>
>> Modern programmers do refactoring and testing.  I'm more than happy with
>> this.  I want our code to be readable and using idiomatic style goes a
>> long way to achieving that.  Working is important but being maintainable
>> is important too.
>>
>> +1 for making such changes but I'd prefer to see them as distinct
>> patches not part of broader changes so it is easier to verify that they
>> really are syntactic refactorings.
>>
>
> Sounds good. If I find myself cleaning up code en route to fixing it, I'll
> try to make a habit of submitting the non-functional change as an
> intermediate step.
>

+1 for this approach. My opinion is that refactoring is fine as long
as there is a high amount of testing - the refactoring in most cases
should just make the code more efficient, simple or easier to read. I
do think refactoring should be separated from bug fixes and
enhancements when possible - it makes watching the changes easier.

-Nathan

Re: Idiomatic Java: return-at-method-end in Harmony

Posted by Jesse Wilson <je...@google.com>.
On Mon, Oct 26, 2009 at 3:44 PM, Mark Hindess
<ma...@googlemail.com>wrote:

> Modern programmers do refactoring and testing.  I'm more than happy with
> this.  I want our code to be readable and using idiomatic style goes a
> long way to achieving that.  Working is important but being maintainable
> is important too.
>
> +1 for making such changes but I'd prefer to see them as distinct
> patches not part of broader changes so it is easier to verify that they
> really are syntactic refactorings.
>

Sounds good. If I find myself cleaning up code en route to fixing it, I'll
try to make a habit of submitting the non-functional change as an
intermediate step.

Re: Idiomatic Java: return-at-method-end in Harmony

Posted by Mark Hindess <ma...@googlemail.com>.
In message <c3...@mail.gmail.com>,
Alexey Petrenko writes:
>
> 2009/10/27 Gregory Shimansky <gs...@apache.org>:
> >
> > On 27 October 2009 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.
> >
> > I don't think anyone who writes C/C++ cares about it. Usually
> > such style is a result of changes upon changes on some code with
> > intention to make minimal changes to it.
> >
> > > 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.
> >
> > If it works, don't fix it.
>
> Right... old programmers truth...

Modern programmers do refactoring and testing.  I'm more than happy with
this.  I want our code to be readable and using idiomatic style goes a
long way to achieving that.  Working is important but being maintainable
is important too.

+1 for making such changes but I'd prefer to see them as distinct
patches not part of broader changes so it is easier to verify that they
really are syntactic refactorings.

Regards,
 Mark.



Re: Idiomatic Java: return-at-method-end in Harmony

Posted by Alexey Petrenko <al...@gmail.com>.
2009/10/27 Gregory Shimansky <gs...@apache.org>:
> On 27 October 2009 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.
>
> I don't think anyone who writes C/C++ cares about it. Usually such style is a
> result of changes upon changes on some code with intention to make minimal
> changes to it.
>
>> 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.
>
> If it works, don't fix it.
Right... old programmers truth...

Alexey

Re: Idiomatic Java: return-at-method-end in Harmony

Posted by Gregory Shimansky <gs...@apache.org>.
On 27 October 2009 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.

I don't think anyone who writes C/C++ cares about it. Usually such style is a 
result of changes upon changes on some code with intention to make minimal 
changes to it.

> 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.

If it works, don't fix it.

-- 
Gregory