Search code examples
javaarraysstringnullfrequency

Why is Java string array appending a string to null in first cell?


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

Solution

  • 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.