patternjavaModerate
Collatz Sequence
Viewed 0 times
collatzsequencestackoverflow
Problem
I'm a beginner and I only know limited amounts such as
if, while, do while. So I'm here to check if I'm coding with best practise and the most effective methods to my current knowledge.import java.util.Scanner;
public class SixtyTwo {
public static void main(String[] args) {
Scanner keyboard = new Scanner(System.in);
System.out.print("Starting Number: ");
int n = keyboard.nextInt();
int counter = 0;
int stepsTaken = 0;
int largestNumber = 0;
System.out.println();
while ( n != 1 ){
if ( ( n & 1 ) == 0 ) {
System.out.print( (n = ( n / 2 )) + " " );
stepsTaken++;
counter++;
} else {
System.out.print( (n = ( n * 3 ) + 1) + " " );
stepsTaken++;
counter++;
}
if ( n > largestNumber ){
largestNumber = n;
}
if (counter == 9){
counter = 0;
System.out.print("\n");
}
}
System.out.println();
System.out.println("\nTerminated after " + stepsTaken + " steps.");
System.out.println("The largest value was " + largestNumber + ".");
}
}Solution
That is some overall quite good code you have there, but I have a couple of comments.
-
-
Your class is called
-
-
-
-
-
Scanner should be closed when you are done with the input. Simply call keyboard.close(); when you have acquired the input that you need.-
Your class is called
SixtyTwo but I have no clue what that has to do with anything. A better name would be Collatz-
if ( ( n & 1 ) == 0 ) although it is a nice way of checking if a number is even, it is for Java programmers more readable to use the % (modulo) operator. I would use if (n % 2 == 0).-
System.out.print( (n = ( n / 2 )) + " " ); ...now this I'm not a big fan of. Sure, it works to both modify a value and outputting it on the same line, but I would separate the assignment to it's own line. Assign on one line, output on another, makes things much clearer to read.-
stepsTaken++; and counter++; don't need to be inside the if-else blocks, put them outside them as they are always done no matter if your condition is true or false.-
if (counter == 9) can instead be if (stepsTaken % 9 == 0), which would remove the need for the counter variable entirely.Context
StackExchange Code Review Q#46034, answer score: 11
Revisions (0)
No revisions yet.