Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

The following is code I wrote to solve a codility coding test. This scored 77% on correctness & 50% on performance. I need help in improving it further.

Problem statement:

Given a non-empty zero-indexed array A consisting of N integers, Write a function which returns maximum distance between indices of this array that have adjacent values. The function should return -1 if no such adjacent values exist.

Definition of adjacent values: Given an array A, a pair of indices (P, Q) is said to have adjacent values if no value in the array lies strictly between values A[P] & A[Q] and in addition A[P]!=A[Q].

Example: When int[] A={1, 4, 7, 3, 3, 5}, the function should return value 4.

Reason: due to adjacent indices (0,4) which is maximum

Complexity requirements:

  • Expected worst case time complexity: \$O(n*log(n))\$
  • Expected worst case space complexity is \$O(n)\$, beyond input storage not counting the storage required for input arguments
  • Elements of input array A can be modified.

Edit: corrected max variable initialization

public static void main(String[] args) {
    int[] a = {1, 4, 7, 3, 3, 5};
    System.out.println(solution(a));        
}


public static int solution(int[] A) {

    //create a set with all distinct integers of the array
    //TreeSet for ordered accessing
    Set<Integer> sortedSet = new TreeSet<Integer>();
    for(int i : A){
        sortedSet.add(i);
    }

    //Two arrays to store max & min index of each 
    //distinct integer of the given array A
    int size =  sortedSet.size();
    int[] maxIndex =  new int[size];
    int[] minIndex =  new int[size];

    //store min & max index of each distinct integer
    int i = 0;
    for(Integer n : sortedSet){
        minIndex[i] = findMinIndex(A, n);
        maxIndex[i] = findMaxIndex(A, n);
        i++;
    }

    //return -1 when no adjacent indices exist
    //Changed max initialization from 0 to -1
    //to correct corner case of array having only one distinct element
    //does this make the correctness 100%?  
    int max = -1;
    int length = minIndex.length;

    //calculate max distance between adjacent integers
    for(int j=1; j<length ;j++ ){
        max = Math.max(max, 
                Math.max( Math.abs(maxIndex[j]-minIndex[j-1]), Math.abs(minIndex[j]-maxIndex[j-1])) );
    }

    return max;
}


private static int findMaxIndex(int[] a, int n) {
    int length = a.length;

    for(int i = length-1 ; i >= 0; i--){
        if(a[i] == n){
            return i;
        }
    }
    return 0;
}


private static int findMinIndex(int[] a, int n) {
    int length = a.length;

    for(int i = 0 ; i < length; i++){
        if(a[i] == n){
            return i;
        }
    }
    return 0;
}
share|improve this question

closed as off-topic by 200_success Nov 22 '15 at 7:10

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

    
The full problem statement can be found at the below link link – Vinod Akkepalli Nov 22 '15 at 6:00
1  
A correctness score of 77% indicates that the code is not ready for review. – 200_success Nov 22 '15 at 7:12
    
@200_success From the tour, I observed that it is allowed to post question regarding "Correctness in unanticipated cases" – Vinod Akkepalli Nov 22 '15 at 8:07
1  
It's not really unanticipated if you already know that it's 23% wrong. – 200_success Nov 22 '15 at 8:08
1  
"does this make the correctness 100%?" -> you shouldn't ask us that. Submit it again to see. But your solution looks slow: what do you think is the time complexity of findMinIndex ? – janos Nov 22 '15 at 10:32