1

Here is my class below, that compares elements in two string arrays, and returns the word with the highest frequency in both arrays. However as visible from the output, the first index is appending none to null in spite of initializing both arrays with the String none. Can someone kindly let me know what I am doing wrong that is leading to this?

public class HelloWorld{
    
    public String[] pro;
    public String[] con;
    String proSplitter;
    String conSplitter;
    
    public HelloWorld() {
        this.pro = new String[9];
        this.con = new String[9];
        for(int i=0;i<this.pro.length;i++)
        {
            this.pro[i]="none";
            this.con[i]="none";
        }
    }

    public String[] getPro() {
        return pro;
    }

    public String[] getCon() {
        return con;
    }

    public void setPro(String pros, int proIndex) {
        pro[proIndex] = pros;
    }

    public void setCon(String cons, int conIndex) {
        con[conIndex] = cons;
    }
    
    public String[] proWord(){
        for(int i=0;i<9;i++)
        {
            proSplitter = proSplitter + pro[i] + ",";
        }
        for(int i=0;i<9;i++)
        {
            conSplitter = conSplitter + con[i] + ",";
        }
        String[] values = proSplitter.split(",");
        for(int i=0;i<values.length;i++)
        {
            values[i] = values[i].trim();
        }
        String[] values1 = conSplitter.split(",");
        for(int i=0;i<values1.length;i++)
        {
            values1[i] = values1[i].trim();
        }

        int [] fr = new int [values.length];
        int visited = -1;

        for(int i = 0; i < values.length; i++){
            int count = 1;
            for(int j = i+1; j < values.length; j++){
                if(!values[i].equalsIgnoreCase("none"))
                {
                    if(values[i].compareTo(values[j])==0){
                        count++;
                        //To avoid counting same element again
                        fr[j] = visited;
                    }
                }
            }
            if(fr[i] != visited)
                fr[i] = count;
        }

        int max = fr[0];
        int index = 0;

        for (int i = 0; i < fr.length; i++)
        {
            if (max < fr[i])
            {
                max = fr[i];
                index = i;
            }
        }

        int [] fr1 = new int [values1.length];
        int visited1 = -1;

        for(int i = 0; i < values1.length; i++){
            int count1 = 1;
            for(int j = i+1; j < values1.length; j++){
                if(!values1[i].equalsIgnoreCase("none"))
                {
                    if(values1[i].compareTo(values1[j])==0){
                        count1++;
                        //To avoid counting same element again
                        fr1[j] = visited1;
                    }
                }
            }
            if(fr1[i] != visited1)
                fr1[i] = count1;
        }
        for(int i = 0;i<values.length;i++)
        {
            System.out.println("pro = "+values[i]);
        }
        for(int i = 0;i<values1.length;i++)
        {
            System.out.println("con = "+values1[i]);
        }
        int max1 = fr1[0];
        int index1 = 0;

        for (int i = 0; i < fr1.length; i++)
        {
            if (max1 < fr1[i])
            {
                max1 = fr1[i];
                index1 = i;
            }
        }

        String sentence[] = new String[2];
        if(values[index].equalsIgnoreCase(values1[index1])) {
            sentence[0] = "balanced";
        }else {
            sentence[0] = values[index];
            sentence[1] = values1[index1];
        }
        return sentence;
    }
    public static void main(String[] args){
        
        HelloWorld tracker = new HelloWorld();
        
        tracker.setPro("Apple, Pear", 1);
        tracker.setCon("Banana", 1);
        tracker.setPro("Apple", 2);
        tracker.setCon("Water Melon", 2);
        tracker.setPro("Guava", 3);
        tracker.setCon("Ball", 3);
        tracker.setPro("Apple", 4);
        tracker.setCon("Mango, Plum", 4);
        
        String[] arr = tracker.proWord();
        System.out.println("pro = "+arr[0]);
        System.out.println("con = "+arr[1]);
    }
}

The output being generated is :

pro = nullnone
pro = Apple
pro = Pear
pro = Apple
pro = Guava
pro = Apple
pro = none
pro = none
pro = none
pro = none
con = nullnone
con = Banana
con = Water Melon
con = Ball
con = Mango
con = Plum
con = none
con = none
con = none
con = none
pro = Apple
con = nullnone
  • 2
    Try initializing `proSplitter`and `conSplitter` with an empty `String` instead of `null` . – Arnaud Aug 10 '20 at 08:18
  • [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173) – Andy Turner Aug 10 '20 at 08:26
  • It would be easier to write a method to find the most frequent thing in *an* array; then just invoke that same method for both the pro and con arrays. (This can be done easily, but inefficiently, using `Collections.frequency(Arrays.asList(array), element)` for each element in the list). – Andy Turner Aug 10 '20 at 08:28
  • @Arnaud wow, that was such an obvious error. Thanks man! – Nirmalya Basu Aug 10 '20 at 08:37

1 Answers1

1

As mentioned by Arnaud, the immediate problem is that you're leaving proSplitter uninitialized, so its value is null. Then, when you come to append a string to it with proSplitter = proSplitter + pro[i] + ",";, proSplitter will be converted (effectively) to "null", and then stuff is appended to the end. So, instead, make it "" initially.

However, you've got another problem here, which is that you're mutating a member variable each time you invoke that method - so it's not null (or empty) second time around, it still contains what was there previously.

The fix for that is straightforward: instead of using a member variable, declare these as local variables.

You've also got the problem that you're effectively duplicating the code to count the most frequent thing in an array: this is what methods are for, to allow you to run the same code over different inputs.

You can also make use of library methods. For example:

String mostFrequent(String[] array) {
  int maxFreq = 0;
  String maxFreqS = "";
  for (String s : array) {
    if (s.equalsIgnoreCase("none")) continue;

    int freq = Collections.frequency(Arrays.asList(array), s);
    if (freq > maxFreq) {
      maxFreq = freq;
      maxFreqS = s;
    }
  }
  return maxFreqS;
}

(There are lots of inefficiencies here. The point is more about writing this as a method, to remove the duplication).

Then you can use this inside your existing method, and it will be a whole lot easier for others - and you - to read.

Andy Turner
  • 137,514
  • 11
  • 162
  • 243
  • Absolutely, I have accepted Arnaud's answer and thanks for elaborating again. – Nirmalya Basu Aug 10 '20 at 08:38
  • Thanks for the detailed answer, I am using methods in original code, this is just a rough sketch of the code, to avoid breaking my company policies, when posting on public forum :) – Nirmalya Basu Aug 10 '20 at 08:48