4
\$\begingroup\$

findSecondLargestNumberInTheArray() method returns the second highest number from an array.

Could you please review this code to enhance performance.

public static int findSecondLargestNumberInTheArray(int array[]) {
    // Initialize these to the smallest value possible
    int highest = Integer.MIN_VALUE;
    int secondHighest = Integer.MIN_VALUE;

    // Loop over the array
    for (int i = 0; i < array.length; i++) {

        // If current element is greater than highest
        if (array[i] > highest) {

            // assign second highest element to highest element
            secondHighest = highest;

            // highest element to current element
            highest = array[i];
        } else if (array[i] > secondHighest)
            // Just replace the second highest
            secondHighest = array[i];
    }

    // After exiting the loop, secondHighest now represents the second
    // largest value in the array
    return secondHighest;
}
\$\endgroup\$
4
  • \$\begingroup\$ What test cases did you try? Do you have a more complete problem statement? As Martin indicates, this code doesn't work when the highest value occurs multiple times. \$\endgroup\$ Commented Jul 26, 2018 at 13:46
  • 1
    \$\begingroup\$ @Mast: As Zeta points out below, that depends on the specification. \$\endgroup\$ Commented Jul 26, 2018 at 13:57
  • \$\begingroup\$ @MartinR Which is why I asked for a more complete problem statement :-) \$\endgroup\$ Commented Jul 26, 2018 at 14:04
  • 1
    \$\begingroup\$ An array with 0 or 1 elements does not have a "Second Highest" so you should throw an error at the start for those cases. Given that the array has at least two elements, then you can avoid using Integer.MIN_VALUE and instead populate the initial values of highest and secondHighest from the first two values in the array in the appropriate order. Then start your loop from i = 2 to process the rest of the array. A two element array will not enter the first loop iteration. \$\endgroup\$ Commented Jul 29, 2018 at 12:12

2 Answers 2

Reset to default
4
\$\begingroup\$

Several comments are unnecessary:

    // Loop over the array
    for (int i = 0; i < array.length; i++) {

    // assign second highest element to highest element
    secondHighest = highest;

I recommend do use braces { ... } for all if-blocks, even if that consists only of a single statement, and definitely if there is an additional comment line:

        } else if (array[i] > secondHighest)
            // Just replace the second highest
            secondHighest = array[i];

Iteration over all array elements can be simplified with an “enhanced for loop:”

    for (int elem : array) {
        // ...
    }

Your function returns Integer.MIN_VALUE for an empty or single-element array, which is fine if specified as such. An alternative would be to throw an exception for arrays with less than two elements.

With respect to performance, I doubt there is much to improve. The function iterates once over all array elements, which looks pretty much optimal.

\$\endgroup\$
2
  • \$\begingroup\$ I'd argue that the problem is currently underspecified. Whether 2 or 1 should get returned on 1, 2, 2 depends on the task at hand. If we define "second highest" as "the penultimate item in the sorted list" (which is 1, 2, 2), then 2 is definitely the correct answer. if we define it as "penultimate item in the sorted list with unique values" (which is 1, 2), well, then 1 would be correct. But I guess the actual task probably contains only unique values and therefore this ambiguity doesn't change the result. \$\endgroup\$ Commented Jul 26, 2018 at 13:51
  • \$\begingroup\$ @Zeta: You are right! – With the first interpretation, OP's code is correct. I'll remove that part. \$\endgroup\$ Commented Jul 26, 2018 at 13:54
-2
\$\begingroup\$ 
public static int findSecondLargestNumberInTheArray(int array[]) {
// Initialize these to the smallest value possible
int highest = Integer.MIN_VALUE;
int secondHighest = Integer.MIN_VALUE;

// Enhanced ForLoop 
for (int elements: array ) {

    // If current element is greater than highest
    if (elements > highest) {
        secondHighest = highest;
        highest = elements;
    } else if (elements > secondHighest && elements < highest)

        secondHighest = elements;
}

//Returning the second Highest from the arrays of elements
return secondHighest;

}

** Made changes in your logic so that it gonna give correct results **

  • and use of for loop will somewhat increase the performance*
\$\endgroup\$
4
  • \$\begingroup\$ You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and why it is better than the original) so that the author and other readers can learn from your thought process. \$\endgroup\$ Commented Jul 26, 2018 at 16:03
  • \$\begingroup\$ @Dannnno I dint gave the alternate solution, I just fixed the logical issue what the code having. And in case of the origina code what we are discussing on will not give the result if they compare like else if (array[i] > secondHighest) // Just replace the second highest secondHighest = array[i]; } Then the maxElement of the array will alway gonna replace the secondMax value . So,made changes so that if the array[i], an element is less than the max value then only it gonna update or else no update in secondMax value . \$\endgroup\$ Commented Jul 26, 2018 at 16:18
  • \$\begingroup\$ @ManjeetKumar: That depends on how “second largest element” is defined, compare Zeta's comment above. If “second largest element” is the equivalent of ”after sorting the array in decreasing order, take the second element” then OP's code is correct. \$\endgroup\$ Commented Jul 26, 2018 at 16:23
  • \$\begingroup\$ @MartinR I too agree with your point of view, but the correct answer is always the unique value which is the second maximum, among all elements. So I made changes in the logic so that it gonna work in all case. And I suggest using Enhanced-ForLoop instead of a normal one. That's what original post is all about. \$\endgroup\$ Commented Jul 26, 2018 at 16:44

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.