Printing the right diagonal of a matrix

I am solving a coding challenge whose objective is to print the right diagonal of a matrix. And below is my code to do that.

```
package basics;
import java.util.*;
/**
Problem statement : Print right diagonal of a matrix. Example: For input : a b, c d ; the right diagonal is b c
**/
public class PrintRightDiagonal{

private int rowSize;
private int colSize;
private char[][] matrix;
private Scanner scanner = new Scanner(System.in);
private String[] rows;
private String userInput;

public static void main(String[] args){
PrintRightDiagonal printRightDiagonal = new PrintRightDiagonal();
printRightDiagonal.readMatrixDimensions();
printRightDiagonal.readInputEntries();
printRightDiagonal.validateInputEntries();
printRightDiagonal.insertIntoArr();
printRightDiagonal.findRDiagonal();
}

private void readMatrixDimensions(){
System.out.println("Please enter the rowsize of the 2D array");
rowSize = scanner.nextInt();
System.out.println("Please enter the colsize of the 2D array");
colSize = scanner.nextInt();
if(rowSize!=colSize){
System.err.println("Cannot find the diagonal of a matrix because the rowsize and column size are different");
System.exit(0);
}
}

private void readInputEntries(){
System.out.println("Please enter the rows separated by comma. Example if the matrix is a 2x2 enter i/p as ab,cd");
scanner.nextLine();
userInput = scanner.nextLine();
}

private void validateInputEntries(){
rows = userInput.split(",");
if(rows.length!=rowSize){
System.err.println("Entered rows doesn't match with the rowSize");
System.exit(0);
}
for(String row : rows){
if(row.length()!=colSize){
System.err.println("Entered cols doesn't match with the colsize");
System.exit(0);
}

Sequential cohesion

From Code Complete, chapter 7, High-quality routines:


Sequential cohesion exists when a routine contains operations that must be performed in a specific order, that share data from step to step, and that don’t make up a complete function when done together.

Consider these lines in the code:

printRightDiagonal.readMatrixDimensions();
printRightDiagonal.readInputEntries();
printRightDiagonal.validateInputEntries();
printRightDiagonal.insertIntoArr();
printRightDiagonal.findRDiagonal();

If you reorder these calls, the program will compile, but it won't work correctly. This is undesirable.

Consider this alternative:

Matrix matrix = readMatrix(new Scanner(System.in));
matrix.printRightDiagonal();

Where readMatrix does:

int size = readMatrixSize(scanner);
char[][] data = readMatrixEntries(scanner, size);
return new Matrix(data);

You can guess what readMatrixSize and readMatrixEntries do.
The point is, each function does one thing (functional cohesion),
the relationship between statements is clear,
and reordering the statements will result in compile errors,
protecting you from runtime errors.

Encapsulation

The idea of encapsulation is limiting the scope of low-level details (information hiding), to reduce the mental burden of understanding a program.

All these fields in the class are a mental burden:

private int rowSize;
private int colSize;
private char[][] matrix;
private Scanner scanner = new Scanner(System.in);
private String[] rows;
private String userInput;

They are a mental burden because these variables are accessible in the entire class, so if you want to know where they are read or modified, you have to read the entire program.

The fields rows and userInput are the worst offenders,
because they are only used during input.
They should not be exposed like this to the entire class,
but encapsulated by the input mechanism, hidden from the rest of the program.

Side effects, semantic rules, failure of encapsulation

This method suggests a deep flaw in the program:

private void readInputEntries(){
    System.out.println("Please enter the rows separated by comma. Example if the matrix is a 2x2 enter i/p as ab,cd");
    scanner.nextLine();
    userInput = scanner.nextLine();
}

What is the scanner.nextLine(); doing there?
It's there because in the earlier readMatrixDimensions call,
after reading the matrix dimensions,
the end of line character is not read.
This is a big problem, for several reasons:

-
Huge burden on the reader. In order to understand the purpose of this line, you have to read the caller of this method, and the calls before this method that affected scanner.

-
readMatrixDimensions is clearly not doing its job. It leaves the scanner in a state that's not ready for the next use. It forces the rest of the program to understand and remember the low-level details of its implementation.

-
If you remove this line, the method will look correct, perfectly fine, even though the program will be broken.

The fix is relatively simple. Make sure that methods don't have such invisible rules, hidden dependencies on the implementation details outside of them. In this example, if you move scanner.nextLine() to readMatrixDimensions, this problem disappears.

Usability, logic

There's a mix of usability and logical issues here:

System.out.println("Please enter the rowsize of the 2D array");
rowSize = scanner.nextInt();
System.out.println("Please enter the colsize of the 2D array");
colSize = scanner.nextInt();
if(rowSize!=colSize){
    System.err.println("Cannot find the diagonal of a matrix because the rowsize and column size are different");
    System.exit(0);
}

The messages talk about a 2D array, but that's an implementation detail. The data of a matrix could be stored in a 1D array.
Users don't have to know such low-level implementation details.

The program is designed to work with a square matrix.
So why give the user the opportunity to enter different row and column sizes? Ask to input a single size value. That will effectively prevent the user from entering invalid values.

The validation is insufficient. 0 or negative values will not work out well.

Exiting the program in case of invalid input is not very user friendly.
Consider asking until there is a valid value, for example:

private int readMatrixSize(Scanner scanner) {
    System.out.println("Please enter the size of the matrix");
    int size;
    while (true) {
        size = scanner.nextInt();
        if (size > 0) {
            break;
        }
        System.out.println("The size must be positive. Try again!");
    }
    // read the end of line character
    scanner.nextLine();
    return size;
}

Single responsibility principle

insertIntoArr does two things:

• Creates a matrix

• Prints the matrix

These should be separated into createMatrix and printMatrix methods.

Naming

findRDiagonal prints the diagonal. The

StackExchange Code Review Q#124599, answer score: 6