Array To Matrix

Question

I made a method that takes an array and converts it to a matrix, given that it is a valid conversion. However, there is some redundant code in the beginning and I thought maybe you could help me? I am new to programming so avoid complicated imports and such.

    private int[][] toMatrix(int[] arr) {

    if(validConversion(arr)){
        int index = 0;
        //redundant
        int row = (int) Math.sqrt(arr.length);
        int col = (int) Math.sqrt(arr.length);

        int[][] matrix = new int[row][col];
        for(int i = 0; i<row; i++){
            for(int j = 0; j<col; j++){
                matrix[i][j] = arr[index];
                index++;
            }
        }
        return matrix;
    }
    return null;
}


boolean validConversion(int[] arr) {
    return arr.length == 1 || arr.length % Math.sqrt(arr.length) == 0;
}

Answer 1

private int[][] toMatrix(int[] arr) {

Don't shorten names just because you can.

---

if(validConversion(arr)){

This does not need to be an extra function, actually, especially if you drop the "1" special case. An array of one item can technically be converted to a matrix with one item...it's just not much of a matrix.

int matrixSize = (int) Math.sqrt(array.length);

if ((array.length % matrizSize) == 0) {

You can also reuse matrixSize now.

---

        for(int i = 0; i<row; i++){
            for(int j = 0; j<col; j++){

I'm a very persistent advocate for that you're only allowed to use single-letter variable names when dealing with dimensions. In this case, using column and row would make the code very nicely readable:

for (int row = 0; row < matrixSize; row++) {
    for (int column = 0; column < matrixSize; column++) {
        matrix[row][column] = array[index];
        index++;
    }
}

index++ could be rolled into the assignment:

matrix[row][column] = array[index++];

I'm torn whether that actually improves readability or not, though.

---

boolean validConversion(int[] arr) {

Members and functions without an explicit visibility modifier are package-private. In my opinion, package-private should be avoided as it easily can lead to unhealthy couplings between classes in the same package.

---

Consider whether you want to perform additional error checks and throw IllegalArgumentExceptions if they don't add up, for example when the given array is null and so on.

Answer 2

We could evaluate the square root just once:

    int width = (int)Math.sqrt(arr.length);
    if (width * width != arr.length) {
        // not square - invalid
        return null;
    }

    int height = width  // not really needed, since we know matrix is square

It's not clear why you reject arrays of 0 or 1 elements, so I removed that test.

We should consider throwing an exception (perhaps InvalidArgumentException?) rather than returning a null object when the test fails.