Commonmark migration
Source Link

#Formatting

Formatting

#USE PROPER INDENTATION - Please!

USE PROPER INDENTATION - Please!

###Braces

Braces

###Close Scanner

Close Scanner

###Unnecessary outer while

Unnecessary outer while

###Use the length of the array

Use the length of the array

###Slight simplification

Slight simplification

###Variables

Variables

###Extract Method

Extract Method

###Speed Improvement

Speed Improvement

#Formatting

#USE PROPER INDENTATION - Please!

###Braces

###Close Scanner

###Unnecessary outer while

###Use the length of the array

###Slight simplification

###Variables

###Extract Method

###Speed Improvement

Formatting

USE PROPER INDENTATION - Please!

Braces

Close Scanner

Unnecessary outer while

Use the length of the array

Slight simplification

Variables

Extract Method

Speed Improvement

added 563 characters in body
Source Link
Simon Forsberg
  • 57.9k
  • 9
  • 149
  • 304

###Speed Improvement

Currently you're saving the result of one calculation into your array, and while that's a good start you are missing out on some values you can use.

Let's take the example of 3. The sequence is: 3, 10, 5, 16, 8, 4, 2, 1

The only result you are storing from this operation is 3 --> 7. But you have also found out that 10 has the value 7 - 1, 5 has the value 7 - 2, 16 has the value 7 - 3 and so on... Your algorithm would increase significantly in speed if you were also storing these intermediate values.


###Speed Improvement

Currently you're saving the result of one calculation into your array, and while that's a good start you are missing out on some values you can use.

Let's take the example of 3. The sequence is: 3, 10, 5, 16, 8, 4, 2, 1

The only result you are storing from this operation is 3 --> 7. But you have also found out that 10 has the value 7 - 1, 5 has the value 7 - 2, 16 has the value 7 - 3 and so on... Your algorithm would increase significantly in speed if you were also storing these intermediate values.

Source Link
Simon Forsberg
  • 57.9k
  • 9
  • 149
  • 304

#Formatting

#USE PROPER INDENTATION - Please!

If you're using Eclipse, press Ctrl + Shift + F.

If you're using Netbeans or IntelliJ, press Alt + Shift + F.

If you're not using any IDE, start doing so!


###Braces

Once your code is properly formatted it can be seen that you're not using braces here:

if (j < 50000000)
    arr[(int) (j - 1)] = count;

It is especially important to use braces when you're code's not formatted properly, as it is easy to accidentally add an extra statement inside the if, which would cause unexpected results.

if (j < 50000000) {
    arr[(int) (j - 1)] = count;
}

###Close Scanner

Scanners should be closed when they have been used. sc.close(); at the end of your program will take care of that.


###Unnecessary outer while

Your outermost while "loop" is only executed once because there is a break; at the end of it. This could be an if instead and the break; at the end removed.


###Use the length of the array

if (j < 50000000) {

and

if (n < 50000000) {

Should instead be:

if (j < arr.length) {

and

if (n < arr.length) {

respectively.


###Slight simplification

if (n % 2 == 0) {
    n = n / 2;
    count++;
}
else {
    n = (3 * n) + 1;
    count++;
}

Can be written as:

n = (n % 2 == 0 ? n / 2 : (3 * n) + 1);
count++;

Or at least, if you don't like the conditional operator (... ? ... : ...), use the if-else for n but do count++ before or after the if/else.


###Variables

Your naming leaves some things to be desired. x and y for example should be named startTime and endTime, the current names makes it sound like it is related to coordinates, which it is not.

The only variable with any meaning is max and count. It is hard to tell what kind of max max is though, naming it maxCycles would make that clearer.

As for your other variables, long i, j, n, pos, I certainly cannot tell what they're used for without looking at your code for a while. These variable names tell me absolutely nothing!

To get a better overlook of the variables, it is better to declare the variables as close to their usage as possible. y (endTime) is declared at the top, but not used until the end of the method.


###Extract Method

Your main method is doing way too many things right now.

Extract parts of your code to different methods:

For example, your entire while (n != 1) loop can be extracted to a determineCycles method.