Skip to main content
added 5 characters in body
Source Link
Uri Agassi
  • 6.7k
  • 1
  • 18
  • 48

Mind your scope
You have a lot of static members (cSum, rSum, etc.), which you calculate in one method, and thethen pass them as arguments to other methods. This means that they should not be members at all - they should be declared locally, and not be exposed outside the method. This way you can be sure no-one else is changing them when you are not looking...

What should a main method do?
Your main method contains a lot of functionality. I suggest you move this functionality to another method (checkBoard()). This way, when you decide to take this code, and use it as a module in another application, you can simply do it!

Keep it DRY
As a programmer, you should have bells ringing in your head each time you find yourself using Copy+Paste. Say you find a bug in your code, now you have to fix it 9 times! and if you miss a fix somewhere - good luck finding where it is...
Take the similarities between your snippets, and refactor them out. Analyze the differences, and pass them as arguments. Find how you decide which argument values to pass, and develop an algorithm to make that decision for you:

for(int i=0 ; i< sMatrix.length ; i++){
   for(int j=0 ; j<sMatrix.length ; j++){
       int boxIndex = (i / 3) * 3 + j / 3;
       boxSumArray[boxIndex]+=sMatrix[i][j];
   }
}

Mind your scope
You have a lot of static members (cSum, rSum, etc.), which you calculate in one method, and the pass them as arguments to other methods. This means that they should be members at all - they should be declared locally, and not be exposed outside the method. This way you can be sure no-one else is changing them when you are not looking...

What should a main method do?
Your main method contains a lot of functionality. I suggest you move this functionality to another method (checkBoard()). This way, when you decide to take this code, and use it as a module in another application, you can simply do it!

Keep it DRY
As a programmer, you should have bells ringing in your head each time you find yourself using Copy+Paste. Say you find a bug in your code, now you have to fix it 9 times! and if you miss a fix somewhere - good luck finding where it is...
Take the similarities between your snippets, and refactor them out. Analyze the differences, and pass them as arguments. Find how you decide which argument values to pass, and develop an algorithm to make that decision for you:

for(int i=0 ; i< sMatrix.length ; i++){
   for(int j=0 ; j<sMatrix.length ; j++){
       int boxIndex = (i / 3) * 3 + j / 3;
       boxSumArray[boxIndex]+=sMatrix[i][j];
   }
}

Mind your scope
You have a lot of static members (cSum, rSum, etc.), which you calculate in one method, and then pass them as arguments to other methods. This means that they should not be members at all - they should be declared locally, and not be exposed outside the method. This way you can be sure no-one else is changing them when you are not looking...

What should a main method do?
Your main method contains a lot of functionality. I suggest you move this functionality to another method (checkBoard()). This way, when you decide to take this code, and use it as a module in another application, you can simply do it!

Keep it DRY
As a programmer, you should have bells ringing in your head each time you find yourself using Copy+Paste. Say you find a bug in your code, now you have to fix it 9 times! and if you miss a fix somewhere - good luck finding where it is...
Take the similarities between your snippets, and refactor them out. Analyze the differences, and pass them as arguments. Find how you decide which argument values to pass, and develop an algorithm to make that decision for you:

for(int i=0 ; i< sMatrix.length ; i++){
   for(int j=0 ; j<sMatrix.length ; j++){
       int boxIndex = (i / 3) * 3 + j / 3;
       boxSumArray[boxIndex]+=sMatrix[i][j];
   }
}
fix modulo
Source Link
Uri Agassi
  • 6.7k
  • 1
  • 18
  • 48

Mind your scope
You have a lot of static members (cSum, rSum, etc.), which you calculate in one method, and the pass them as arguments to other methods. This means that they should be members at all - they should be declared locally, and not be exposed outside the method. This way you can be sure no-one else is changing them when you are not looking...

What should a main method do?
Your main method contains a lot of functionality. I suggest you move this functionality to another method (checkBoard()). This way, when you decide to take this code, and use it as a module in another application, you can simply do it!

Keep it DRY
As a programmer, you should have bells ringing in your head each time you find yourself using Copy+Paste. Say you find a bug in your code, now you have to fix it 9 times! and if you miss a fix somewhere - good luck finding where it is...
Take the similarities between your snippets, and refactor them out. Analyze the differences, and pass them as arguments. Find how you decide which argument values to pass, and develop an algorithm to make that decision for you:

for(int i=0 ; i< sMatrix.length ; i++){
   for(int j=0 ; j<sMatrix.length ; j++){
       int boxIndex = (i %/ 3) * 3 + j %/ 33;
       boxSumArray[boxIndex]+=sMatrix[i][j];
   }
}

Mind your scope
You have a lot of static members (cSum, rSum, etc.), which you calculate in one method, and the pass them as arguments to other methods. This means that they should be members at all - they should be declared locally, and not be exposed outside the method. This way you can be sure no-one else is changing them when you are not looking...

What should a main method do?
Your main method contains a lot of functionality. I suggest you move this functionality to another method (checkBoard()). This way, when you decide to take this code, and use it as a module in another application, you can simply do it!

Keep it DRY
As a programmer, you should have bells ringing in your head each time you find yourself using Copy+Paste. Say you find a bug in your code, now you have to fix it 9 times! and if you miss a fix somewhere - good luck finding where it is...
Take the similarities between your snippets, and refactor them out. Analyze the differences, and pass them as arguments. Find how you decide which argument values to pass, and develop an algorithm to make that decision for you:

for(int i=0 ; i< sMatrix.length ; i++){
   for(int j=0 ; j<sMatrix.length ; j++){
       int boxIndex = i % 3 * 3 + j % 3
       boxSumArray[boxIndex]+=sMatrix[i][j];
   }
}

Mind your scope
You have a lot of static members (cSum, rSum, etc.), which you calculate in one method, and the pass them as arguments to other methods. This means that they should be members at all - they should be declared locally, and not be exposed outside the method. This way you can be sure no-one else is changing them when you are not looking...

What should a main method do?
Your main method contains a lot of functionality. I suggest you move this functionality to another method (checkBoard()). This way, when you decide to take this code, and use it as a module in another application, you can simply do it!

Keep it DRY
As a programmer, you should have bells ringing in your head each time you find yourself using Copy+Paste. Say you find a bug in your code, now you have to fix it 9 times! and if you miss a fix somewhere - good luck finding where it is...
Take the similarities between your snippets, and refactor them out. Analyze the differences, and pass them as arguments. Find how you decide which argument values to pass, and develop an algorithm to make that decision for you:

for(int i=0 ; i< sMatrix.length ; i++){
   for(int j=0 ; j<sMatrix.length ; j++){
       int boxIndex = (i / 3) * 3 + j / 3;
       boxSumArray[boxIndex]+=sMatrix[i][j];
   }
}
Source Link
Uri Agassi
  • 6.7k
  • 1
  • 18
  • 48

Mind your scope
You have a lot of static members (cSum, rSum, etc.), which you calculate in one method, and the pass them as arguments to other methods. This means that they should be members at all - they should be declared locally, and not be exposed outside the method. This way you can be sure no-one else is changing them when you are not looking...

What should a main method do?
Your main method contains a lot of functionality. I suggest you move this functionality to another method (checkBoard()). This way, when you decide to take this code, and use it as a module in another application, you can simply do it!

Keep it DRY
As a programmer, you should have bells ringing in your head each time you find yourself using Copy+Paste. Say you find a bug in your code, now you have to fix it 9 times! and if you miss a fix somewhere - good luck finding where it is...
Take the similarities between your snippets, and refactor them out. Analyze the differences, and pass them as arguments. Find how you decide which argument values to pass, and develop an algorithm to make that decision for you:

for(int i=0 ; i< sMatrix.length ; i++){
   for(int j=0 ; j<sMatrix.length ; j++){
       int boxIndex = i % 3 * 3 + j % 3
       boxSumArray[boxIndex]+=sMatrix[i][j];
   }
}