Searching in Multidimensional Array

Question

I was wondering about the Array built in methods in java 8. As there is no specific method other than Arrays.binarySearch. So for multidimensional array I wrote following method.

public int[] search(Integer[][] mat,int key)
    {
        int r=0,c=0;
        for(Integer[] a:mat)
        {
            if(Arrays.asList(a).contains(key)){
                c = Arrays.asList(a).indexOf(key);
                break;
            }
            else r++;
        }
        return new int[]{r+1,c+1};
    }

checking

 Integer[][] mat = {{1,2,3,99},{4,5,6,57},{7,8,9,89},{10,11,12,13}};
int [] rc = new int[2];
 rc = search(mat, 8);
 System.out.println(Arrays.toString(rc));
 rc = search(mat, 9);
 System.out.println(Arrays.toString(rc));
 rc = search(mat,10);
 System.out.println(Arrays.toString(rc));
 rc = search(mat, 1);
 System.out.println(Arrays.toString(rc));
   

output

[3, 2]
[3, 3]
[4, 1]
[1, 1]

I would like a review of efficiency and general code quality. Also any improvements I can do to make it better.

Answer 1

A few things to mention:

As has been mentioned, you're using 1-based indexing instead of 0-based indexing. 0-based is much more intuitive, since most implementations, of indexing in most programming languages, use 0-based indexing.

You're using the Arrays.asList twice for each iteration of the loop. This increases the inefficiency.

Calling indexOf inside the loop is basically creating another loop and doesn't gain anything a for loop wouldn't give you.

I think the name of your method could be better. search indicates all you want is the value. However you're returning the indexes. Therefore indexesOf might be more appropriate, and more in fitting with similar methods in other collections.

Your method is hard coded for int. This isn't very extensible. A generic version would probably be more fitting.

You're returning an int[]. This doesn't really tell anyone what this is supposed to represent. I would suggest a class(Indexes) to hold the indexes would allow you to represent them with a name that will immediately let the user know what it represents.

You haven't made any provision to handle failed searches.

It looks to me that the 2D array is basically a matrix. Therefore I would suggest labeling the indexes as such(row,column).

The Indexes class would look like this:

public class Indexes {
    int row = 0;
    int col = 0;

    public Indexes(int row, int col) {
        this.row = row;
        this.col = col;
    }

    public Indexes() {
    }

    public int getRow() {
        return row;
    }

    public void setRow(int row) {
        this.row = row;
    }

    public int getCol() {
        return col;
    }

    public void setCol(int col) {
        this.col = col;
    }
    @Override
    public String toString(){
        return String.format("[%d, %d]",row,col);
    }
}

A generified version with the points I mentioned could look something like this:

public static<T> Indexes indexesOf(T[][] arr, T value){
    for(int row = 0; row < arr.length; ++row){
        for(int col = 0; col < arr[row].length; ++col){
            if(arr[row][col].equals(value)){
                return new Indexes(row,col);
            }
        }
    }
    return new Indexes(-1,-1);
}

EDIT Alternative code based on OP's comments

static final int DEFAULT_RETURN_VALUE = -1;
public static<T> Indexes indexesOf(T[][] arr, T value){
    for(int row = 0; row < arr.length; ++row){
            int col = indexOf(arr[row],value);
            if(col > DEFAULT_RETURN_VALUE){
                return new Indexes(row,col);
            }
    }
    return new Indexes(DEFAULT_RETURN_VALUE,DEFAULT_RETURN_VALUE);
}
public static<T> int indexOf(T[] arr, T value){
    for(int i = 0; i < arr.length; ++i){
        if(arr[i].equals(value)){
            return i;
        }
    }
    return DEFAULT_RETURN_VALUE;
}