2D array challenge

Question

I'm doing a hackerrank challenge to get ready for interviews and was wondering if someone could provide feedback on my code (style, naming convention, etc.). This challenge involves 2D arrays and is located here: https://www.hackerrank.com/challenges/2d-array

Here is my code (that passes all the test cases):

import java.io.*;
import java.util.*;
import java.text.*;
import java.math.*;
import java.util.regex.*;

public class Solution {
    private static final int ROWS = 6;
    private static final int COLS = 6;
    private static final int HOURGLASS_ROWS = 3;
    private static final int HOURGLASS_COLS = 3;
    static int[][] arr = new int[ROWS][COLS];
    static int[][] hourglass = new int[][]{{1, 1, 1}, {0, 1, 0}, {1, 1, 1}};

    static int findMaxSum(Scanner sc) {
        int maxSum = Integer.MIN_VALUE;     
        initialize(sc);

        for (int i = 0; i <= ROWS-HOURGLASS_ROWS; i++) {
            for (int j = 0; j <= COLS-HOURGLASS_COLS; j++) {
                int sum = 0;
                for (int k = 0; k < HOURGLASS_ROWS; k++) {
                    for (int l = 0; l < HOURGLASS_COLS; l++) {
                        sum += hourglass[k][l]*arr[i+k][j+l];
                    }
                }
                if (sum > maxSum) maxSum = sum;
            }
        }
        return maxSum;
    }

    static void initialize(Scanner sc) {
        for (int i = 0; i < ROWS; i++) {
            for (int j = 0; j < COLS; j++) {
                arr[i][j] = sc.nextInt();
            }
        }
    }

    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        System.out.print(findMaxSum(sc));
    }
}

Thanks for the feedback!

Please include the challenge description into your question. — Heslacher
– Heslacher, Commented Aug 18, 2016 at 8:50

Pieter Witvoet · Accepted Answer · 2016-08-18 12:41:05Z

I don't use Java a lot, so I can't say too much about style and conventions, but here's what I've got:

initialize isn't a very descriptive name. Something like parse2DArray is better.
It's better to let initialize return a 2D array, and to let findMaxSum take a 2D array as argument. This makes both methods more flexible and reusable (for example, findMaxSum is no longer limited to a specific input format, and initialize/parse can be called by multiple threads at the same time).
initialize can be made more flexible by taking a width and height as arguments. It's not necessary to meet the requirements of this particular challenge, but it takes almost no extra effort and it gives you a method that can be reused for other 2D array challenges.
Creating a 2D array for the hourglass pattern is a good idea. Why not modify findMaxSum to accept a pattern as an argument? Again, more flexibility at no extra cost.
findMaxSum actually does two things at once: it runs through the 2D array, using the hourglass pattern to determine sums. It also keeps track of the maximum sum and returns that. This could be split up into two methods: one that applies the hourglass pattern and returns a list (or Iterable<Integer>) of sums, and another method that takes the maximum value of a given list/iterable.
Putting an if statement and its associated code on the same line makes the code harder to 'scan': at first glance it reads like a single statement, not something that happens conditionally.
Add spaces around operators. For example, ROWS-HOURGLASS_ROWS looks like a single constant name.
Minor naming point: when looping over 2D arrays, I prefer x and y over i and j.

Your code does what it needs to do, and for a challenge like this that's sufficient. However, it heavily relies on 'global' state (statics) and constants instead of passing arguments and returning values, which results in poor reusability. That's no problem in a small program like this, but it's an approach that doesn't scale well to larger and more complex programs, where 'coupling' should be kept as low as possible.

In Java i, j, k etc. are the standard for counter variables — yitzih
– yitzih, Commented Aug 18, 2016 at 13:44
@yitzih If there's a relationship between the counters, it's better to express that relationship in variable names. x and y are great for such thing. — Pimgd
– Pimgd, Commented Aug 18, 2016 at 16:21

Stack Exchange Network

2D array challenge

1 Answer 1

You must log in to answer this question.

Hot Network Questions

2D array challenge

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions