QUEST275
QUEST275

Reputation: 23

This Code of mine should return the frequency of the most common element

Arrays.sort(arr);
int max=1,m=1;
for(int i=1;i<arr.length;i++){
    if(arr[i]==arr[i-1]){
        max++;
    }
    else{
        if(max>m){
            m=max;
            max=1;
        }
       
    }
}
if(max>m){
    m=max;
   
}

return m;

This is a function that I have made. This should return the number of times the most frequent element occurs. E.g if the array is 1,2,2,3,3,3 , then it should return 3. But it fails in many cases, e.g for the input 1 2 3 1 2 3 3 3, this code fails and returns 5, which is the wrong output.

Upvotes: 1

Views: 162

Answers (6)

WJS
WJS

Reputation: 40024

You need to check max against m for each iteration. And the use of continue here simplifies the logic. Prints -1 on an empty array.

int[] arr = {1, 2, 3, 1, 2, 3, 3, 3}; 
Arrays.sort(arr);                       
int max = 1;                            
int m = arr.length == 0 ? -1 : 1;                             
for (int i = 1; i < arr.length; i++) {  
    if (arr[i] == arr[i - 1]) {         
        max++;                          
        if (max > m) {                  
            m = max;                    
        }                               
        continue;                       
    }                                   
    max = 1;                            
}                                       
                                                                         
System.out.println(m);

Prints

4

Upvotes: 0

Gryphon
Gryphon

Reputation: 384

Another way using streams, which basically uses a map to track the frequency of occurrences and grabs the highest value after sorting that map.

public static Long getMostFrequentCount( int ... values ) {
    return Arrays.stream(values).boxed().collect(Collectors.groupingBy(Function.identity(),
            Collectors.counting())).values().stream().max(Long::compareTo).orElse( null );
}

EDIT: Made better thanks to @saka1029 's excellent suggestion

Upvotes: 1

khelwood
khelwood

Reputation: 59096

Sorting the array first and counting runs of the same number is a good idea. Your logic doesn't quite make sense through.

You need to keep track of the length of the current run, and the length of the longest run. Your current run should get reset when the value in the array is different to the previous value; and your longest run should be updated when the current run is longer than it.

Something like this:

if (arr.length==0) {
    return 0;
}
Arrays.sort(arr);
int currentRun = 1;
int longestRun = 1;
for (int i = 1; i < arr.length; i++){
    if (arr[i]==arr[i-1]){
        ++currentRun;
        if (currentRun > longestRun) {
            longestRun = currentRun;
        }
    } else {
        currentRun = 1;
    }
}

return longestRun;

Upvotes: 1

K. hervey
K. hervey

Reputation: 119

I can code it up for you to answer your question using the map interface. I will check each value and if there is already a key for that value, it will increment the value by 1. If not, then it will create the key and assign it a value of 1.

When finished, I only need to ask the map what the largest value was which is what I think you are after. And I can also return the value that was the most frequent, if you want.

Here is the tested code. One class for the working method and then the driver class with main method

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;

public class FindMostFrequent {

    public static Integer returnFromArrayHighestFrequency(int[] inArray) {
        HashMap<Integer, Integer> hashMap = new HashMap<Integer, Integer>();

        for (int i = 0; i < inArray.length; i++) {
            if (hashMap.containsKey(inArray[i])) {
                hashMap.put(inArray[i], hashMap.get(inArray[i]) + 1);
            } else {
                hashMap.put(inArray[i], 1);
            }
        }
        Integer maxValue = Collections.max(hashMap.values());

        return maxValue;
    }

}

And here is the driver class:

    public static void main(String[] args) {
        int[] testArray = { 1, 2, 2, 3, 3, 3 };

        Integer max = FindMostFrequent.returnFromArrayHighestFrequency(testArray);
        System.out.println("highest frequency is:  " + max);

    }

I like this technique because it allows you to easily get the minimum or another other value and its key that you want.

Upvotes: 3

codeNdebug
codeNdebug

Reputation: 249

your logic is totally correct except one line. ==> if(max>m). In this case you are not resetting the value of max if max == m.

replace if(max>m){ with if(max>=m){

Upvotes: 0

HugoS
HugoS

Reputation: 105

This might be what you want... All of the numbers you want to use is contained inside of the array a.

public class Main {
public static void main(String[] args) {
    int[] a = {1,2,3,4,5,6,7,7,7,7};
    int count = 1, tempCount;
    int popular = a[0];
    int temp = 0;
    for (int i = 0; i < (a.length - 1); i++) {
        temp = a[i];
        tempCount = 0;
        for (int j = 1; j < a.length; j++) {
            if (temp == a[j])
                tempCount++;
        }
        if (tempCount > count) {
            popular = temp;
            count = tempCount;
        }
    }
    System.out.println(popular);
}
}

Upvotes: 0

Related Questions