-1
\$\begingroup\$

I wrote a program that finds out if matrix has 2 oblique lines that each oblique line has the same value. The lines does not have to be the same value. for example:
for this example, output is false (there is only 1 oblique of 1,1,1)

for this example, output is true (there is 2 oblique: 3,3,3 and 2,2,2)

This is my code and I would like to get some advise if it's good.

public static void main (String[] args)  {
   String size = StdIn.readLine();
   boolean indicator = false;
   int matrixSize = Integer.parseInt(size);
   int [][] matrix = new int [matrixSize][matrixSize];
   for (int i=0; i <= matrixSize-1; i++) {
      for (int j=0; j <= matrixSize-1; j++) {
           matrix[i][j] = StdIn.readInt();
      }
   }
   for (int k=0; k <= matrixSize-2; k++) {
      for (int l=0; l <= matrixSize-2; l++) {
        if (matrix[k][l] == matrix[k+1][l+1])
          indicator = true;
        if (indicator = true && matrix[k][l] == matrix[k+1][l+1]) {
          indicator = true; }
        else {
          indicator = false;
        }
      }
   }
    StdOut.println(indicator);
 }
\$\endgroup\$
5
  • \$\begingroup\$ How do you define an oblique line? I'm not quite sure if you are using the correct word. Possibly diagonal line would be better, so please define what you consider being an oblique line. \$\endgroup\$
    – holroy
    Nov 21, 2015 at 11:55
  • \$\begingroup\$ @holroy Crossing Diagonal - helps you understand better? \$\endgroup\$
    – dana
    Nov 21, 2015 at 12:13
  • \$\begingroup\$ Better. Do they need to be crossing? And which length do they need to be? In the first you have 2, 2, and 1, 1 going from left up towards right. Would these have been consider as True if length had been three? \$\endgroup\$
    – holroy
    Nov 21, 2015 at 12:16
  • \$\begingroup\$ they Must to be crossing, the length should be at least 2 (which means 2+). It will be still consider true if length will be 3. \$\endgroup\$
    – dana
    Nov 21, 2015 at 13:03
  • \$\begingroup\$ The code does not appear to accomplish the task at all. \$\endgroup\$ Nov 21, 2015 at 17:17

1 Answer 1

1
\$\begingroup\$

I don't know much about the computation logic (and currently don't have time to dive into it), so I will leave that aside and focus on the syntax itself (since we're on CR).

  1. Extract logic into different methods to improve readability, e.g. one for reading the matrix and one for computing the output.

  2. Format your code to improve readability as well.

  3. Use only one style for if-then statements where the clause contains only one statement, i.e.

    if (matrix[k][l] == matrix[k + 1][l + 1])
        indicator = true;
    

    vs

    if (indicator = true && matrix[k][l] == matrix[k + 1][l + 1]) {
        indicator = true;
    }
    

    I suggest the latter because in many times you'll end up with a second statement anyway and it's easy to forget about the brackets then.

  4. The latter shows also a (probably accidential) assignment in your if clause:

    if (indicator = true && ...
    

    should probably be

    if (indicator == true && ...
    

    which you then could also shorten to

    if (indicator && ...
    

    which still wouldn't make much sense, because the other condition (matrix[k][l] == matrix[k+1][l+1]) is exactly the same that you already checked above. And why would you want to set indicator to true when it is already?

  5. I don't know what StdIn and StdOut are, but unless they provide significant benefits over the standard System.in and System.out use the standard ones. Or even better: Use a java.util.Scanner for reading inputs:

    Scanner sc = new Scanner(System.in);
    

    Then you can simply write int i = sc.nextInt(); and don't have to parse the input yourself (cf. also this question on SO).

  6. Integer.parseInt() may throw a NumberFormatException which you might want to handle.

  7. You overwrite the value of indicator in every run of the loop so your output will only contain the result of your last run.

So to sum it all up: No, I don't think your code is good (sorry). In fact you might rather want to post it on Stack Overflow to get the logic straight before improving the code style.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ The best first posts are answers like yours, Marvin. Welcome to CodeReview. \$\endgroup\$
    – Legato
    Nov 21, 2015 at 17:55

Not the answer you're looking for? Browse other questions tagged or ask your own question.