aspose file tools*
The moose likes Beginning Java and the fly likes JavaRanch style: Avoiding use of break /return in the middle of a method Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "JavaRanch style: Avoiding use of break /return in the middle of a method" Watch "JavaRanch style: Avoiding use of break /return in the middle of a method" New topic
Author

JavaRanch style: Avoiding use of break /return in the middle of a method

Luigi Plinge
Ranch Hand

Joined: Jan 06, 2011
Posts: 441

The JavaRanch style guide contains the following two paragraphs:
3.1.2 - Never use return in the middle of a method
return is to be used at the end of a method only.

Reasoning: Using return in the middle of a method makes it difficult to later break the method into smaller methods. It also forces the developer to consider more than one exit point to a method.

3.1.4 - Never use break other than in a switch statement
break is used only for switch statement control.

Reasoning: Using break, other than for switch statement control, makes it difficult to later break a construct into smaller constructs or methods. It also forces the developer to consider more than one end point for a construct.


I'm trying do solve the following problem (from Project Euler):
The number, 1406357289, is a 0 to 9 pandigital number because it is made up of each of the digits 0 to 9 in some order, but it also has a rather interesting sub-string divisibility property.

Let d1 be the 1st digit, d2 be the 2nd digit, and so on. In this way, we note the following:

d2d3d4=406 is divisible by 2
d3d4d5=063 is divisible by 3
d4d5d6=635 is divisible by 5
d5d6d7=357 is divisible by 7
d6d7d8=572 is divisible by 11
d7d8d9=728 is divisible by 13
d8d9d10=289 is divisible by 17

Find the sum of all 0 to 9 pandigital numbers with this property.

Assuming I have a pandigital number that I want to test, I've written the following method to test whether it fits the criterea above:

As you can see, I have a "return false" in the middle of the method. I could instead simply put in a boolean flag that goes to false when a counterexample is detected and return that, and run through all the cases in the inner loop. However we will be calling this method several million times, so performance is important. (Actually for this problem it'll probably be only a difference of a second or two, but it may be more important in similar code for other problems.)

How could I get this to conform to JavaRanch style, without degrading performance?
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39784
    
  28
EasyI have also corrected your indentation.

But why are you using Strings? That is inefficient. Why don't you divide the number directly into its constituent 3-digit numbers? Why are you hard-coding 2 and 8 in the loop? Why are you using <= rather than <?
Luigi Plinge
Ranch Hand

Joined: Jan 06, 2011
Posts: 441

Thanks, that looks like the answer!

Instead of Strings, when you say dividing directly, do you mean something like



Would it make any sense not to hard code 2 and 8 in the loop? I use these numbers to correspond to the problem description and so make the solution more legible, debuggable and maintainable.

I use <= because in other languages a FOR loop is usually described as something like for a = 1 to 10, where 10 is the last value to be evaluated. It makes more sense if you can see what the last value to be evaluated is.
Luigi Plinge
Ranch Hand

Joined: Jan 06, 2011
Posts: 441

In the above I just realized I should do the division first, and then you just need to use modulus 1000 every time... so
Luigi Plinge
Ranch Hand

Joined: Jan 06, 2011
Posts: 441

Did some micro-benchmarking and found the integers version in my second post is just over twice as quick as the Strings version. (There's an error in the Strings version: it should read s.substring(i - 1, i + 2) ).

Then I changed the pows array as follows, switched the modulus and division round and it's twice as fast again!*Having the extra boolean check in there as dictated by good style seems to cost a couple of % points of speed, but it's fairly insignificant.

* edited: had a bug in there so it wasn't completing propery - actually it's only about 1-2% quicker.
Jesper de Jong
Java Cowboy
Saloon Keeper

Joined: Aug 16, 2005
Posts: 14338
    
  22

There are tools, for example FindBugs, Checkstyle and PMD that can analyze your code and warn for possibly dangerous constructs such as putting returns in the middle of a method, or bad style. Those tools are very useful to prevent you from making bugs.

I remember an actual case of a bug I was trying to find, which turned out to be caused by an early return. It was a method that had grown over time, and somebody added some code at the bottom of the method that had to be always executed. But in the beginning of the method, there was an early return, and when that was reached the new code at the bottom was ofcourse not run.


Java Beginners FAQ - JavaRanch SCJP FAQ - The Java Tutorial - Java SE 8 API documentation
fred rosenberger
lowercase baba
Bartender

Joined: Oct 02, 2003
Posts: 11475
    
  16

not to sound like a broken record, but honestly, you shouldn't be worrying about such micro-optimizations. You should focus on writing clean, easy to read, easy to maintain code. This should ALWAYS be your primary goal - and adhering to the coding style of your organization is a big part of that.

Worrying about making it run as fast as possible is counterproductive 99.9999999999% of the time. The code will almost always be fast enough. If you have documented requirements on speed, and if your code doesn't meet it, THEN use a profiler to determine where your code is slow, and focus there - never guess what you think will make it run faster. You will almost never guess right.


There are only two hard things in computer science: cache invalidation, naming things, and off-by-one errors
Luigi Plinge
Ranch Hand

Joined: Jan 06, 2011
Posts: 441

So would you go for the Strings option, Fred? To me this is a more obvious and easier-to-code (requires less thinking) way of breaking a 10-digit number into chunks of 3 digits...
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39784
    
  28
Breaking a ten-digit number up is very easy, with the / and % operators. The only problem is that you might get the rightmost three digits off first. Never mind the speed; it still looks very peculiar to me to convert everything to Strings and back.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39784
    
  28
What you can do is have divisors, and divide the divisors by 10 every time.

Do they really expect you to go through all pandigital numbers? Assuming you don't start with 0, there are 9 × 9! of them.
fred rosenberger
lowercase baba
Bartender

Joined: Oct 02, 2003
Posts: 11475
    
  16

Luigi Plinge wrote:So would you go for the Strings option, Fred?
Personally, I would approach this in an entirely different manner where dividing a number up into sub-digits isn't needed...

But I haven't tackled this one yet, so my initial thoughts may not work at all.

If i were trying to break apart a number, I would not convert it to a string to pick it apart. I would use / and %.
Luigi Plinge
Ranch Hand

Joined: Jan 06, 2011
Posts: 441

Campbell Ritchie wrote:What you can do is have divisors, and divide the divisors by 10 every time.

Do they really expect you to go through all pandigital numbers? Assuming you don't start with 0, there are 9 × 9! of them.


That's only about 3 million numbers, which won't take more than a second or two at most to compute. My problem at the moment is generating the pandigital numbers efficiently. Testing every number for padigitality is inefficient because there are 10 billion numbers to test and that could take a while (although probably not longer than the time taken to code an alternative). I wrote something long and complicated last night which didn't really work but I've thought of a more efficient way... just got to code it now.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39784
    
  28
You can try a recursive or iterative method which loops through the digits and 0. Rather like creating words aaa aab aac ... zzx zzy zzz. It is more complicated because you need to check you aren't using the same digits twice. You could put the individual digits in a Set<Integer>, and if the digit being tried is already in that set, simply skip that iteration. You can start with 10, 12, 13 ... 19, 20, 21, 23 ... 98.
You are right; 9 × 9! = 3265920.
Luigi Plinge
Ranch Hand

Joined: Jan 06, 2011
Posts: 441

Wrote a brute force method for getting the pandigitals that took nearly 2 hours, so tried another way. Amazingly, this worked first time (planned it out on a bit of paper first) - well except I forgot to add the initial pandigital to the list. Takes about 7 seconds. Then another 3 seconds when I add the code to process them all with the isValid method.

If anyone wants to criticise my code, go ahead. It probably looks pretty horrible if you're a functional programmer since the methods have lots of side-effects (updating the instance variables), but well... it seemed like it would be more efficient just to have 1 digits array rather than 3 million of them (Fred will tell me off for this ).

Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39784
    
  28
Who chose that class name?
I don't like the while loop as you have it; I think it should read while (next != 9876543210L) . . .
Luigi Plinge
Ranch Hand

Joined: Jan 06, 2011
Posts: 441

P043 stands for Problem 43 on Project Euler... all my solution classes start with the problem number and are Runnable so I can kick them off from a launcher I wrote. 10pan is a reminder that it's a problem about 10-digit pandigitals... V2 mean's it's version 2. Cryptic maybe but it's hard to describe a problem in a few characters.

Good point about the while loop.
 
jQuery in Action, 2nd edition
 
subject: JavaRanch style: Avoiding use of break /return in the middle of a method