-1
public class StringEx {

public static void main(String[] args){

    String s1 ="abcc";
    String s2 = "cbaa";

    getCommon(s1,s2);
}

private static void getCommon(String s1, String s2) {
    char[] c1 = s1.toCharArray();
    char[] c2 = s2.toCharArray();

    char[] commonAry = new char[10];

    for(int i=0;i < (c1.length)-1;i++){
    for(int j=0;j<(c2.length)-1;j++){

        if(c1[i]==c2[j]){
            int k=0;
            commonAry[k]=c1[i];
            k++; 
        }
    }
    }

    System.out.println(commonAry);

}

}

the above program is giving an output like "c n some square boxer after that".. what is wrong in above code.

SpringLearner
  • 13,738
  • 20
  • 78
  • 116
Siva Naidu
  • 71
  • 1
  • 5

2 Answers2

4

Use Set<character> retainAll() method to get the intersect between the two String

public static void getCommon(String s1,String s2){


         char[] s1Array = s1.toCharArray();  
         char [] s2Array = s2.toCharArray();  


     Set<Character>s1CharSet = new HashSet<Character>();  
     Set<Character>s2CharSet = new HashSet<Character>();  

     for(char c:s1Array){  
         s1CharSet.add(c);  
     }  

     for(char c: s2Array){  
         s2CharSet.add(c);  
     }  

     s1CharSet.retainAll(s2CharSet);  

     if(s1CharSet.size()==0){  
        System.out.println("There are no common characters between the two strings");  
     }  

     else{  
         System.out.println(s1CharSet);  
     }  


     }  

output

  [b, c, a]

Demo

Nambi
  • 11,944
  • 3
  • 37
  • 49
  • Did you actually run this, and get `"abc"` as the output? – Dawood ibn Kareem Feb 01 '14 at 08:43
  • @DavidWallace Yes I get the abc as the output and add the Demo link check it out – Nambi Feb 01 '14 at 09:59
  • OK. You do realise that with different input strings, this isn't going to work out quite as nicely, right? There are at least three bugs, which coincidentally fail to manifest, with this particular input. – Dawood ibn Kareem Feb 01 '14 at 10:14
  • @DavidWallace Thanks, i have found the difficults when I tried by using == comparator ,so i tried it retainAll method in set, can you plz check my edited answer.. – Nambi Feb 01 '14 at 15:25
  • 1
    It looks much, much better now; and as far as I can see, it works correctly. Note that you don't need to iterate through the `char` arrays yourself, to make the sets. You can just write `Set s1CharSet = new HashSet(Arrays.asList(s1Array));` – Dawood ibn Kareem Feb 01 '14 at 17:09
2

firstly, you are not looping over all the char array, your looping should be:

for(int i=0;i < (c1.length);i++)

secondly, you must initialize the index k out of your loops, thirdly, this operation is not commutative, that means

getCommon(String s1, String s2)

and

getCommon(String s2, String s1)

won't give the same results,

and finally, because you're trying to get common letters, so why not break the inner loop just after finding any similar character in the other array?

Community
  • 1
  • 1
Curcuma_
  • 851
  • 2
  • 12
  • 37