Find Longest common subsequence : my method

Question

I am trying to implement the Longest common sub sequence of 2 strings. My method is bit different than posted here. I am near the solution but little problem. Please help me.

My problem: Desired Output: metallica current output: etallica

My code is :

import java.util.ArrayList;
import java.util.Stack;

public class CommonSubS  {

public static void main(String[] args) {

   ArrayList<Character> ss = new ArrayList<>();
   String s1 = "metallicarules";
   String s2 = "rulmetallicales";
   ArrayList<ArrayList<Character>> one = new ArrayList<>();
   char[] first = s1.toCharArray();
   char[] second = s2.toCharArray();
   int j = 0;
   int current = 0;
   int mainIndex = 0;
   ArrayList<Character> sec = new ArrayList<>();

   // search for each char in second            
   for(int i = 0; i<second.length; i++)
   {    
       j = current;
       // search for each char in first
       while(j<first.length)
       {
           // if match found, add to the arraylist
           if(first[j] == second[i]){

               sec.add(first[j]);
               current = j+1;
               break;
           }
           else 
           {   // if different char occured in between,
               // save current arraylist in one
               // and go forward
               one.add(sec);
               sec = new ArrayList<>();
               j++; 
               current = 0;        
           }

       }
   }

   for(ArrayList<Character> s: one)
   {
       for(Character c: s)
       {
           System.out.print(c);
       }
       System.out.println();    
    }

  }
}

/*
 desired output: metallica
current output: etallica
*/

I know there are other methods but I tried to implement using simple method. Please point out the problem.

_Please point out the problem_ ? You are supposed to ask the problem you are facing. — Apurv, Mar 19 '13 at 03:49
I just edited it. My 2 string are "metallicarules" and"rulmetallicales". I need output as metallica. Instead of that I am getting etallica. — Aditya Peshave, Mar 19 '13 at 03:56
What are your `ss` and `mainIndex` variables for? They do not seem to be doing anything. — Alden, Mar 19 '13 at 03:59
@Alden: you are right. sorry about that. Actually I was testing the code. The variables remained there. Sorry :) — Aditya Peshave, Mar 19 '13 at 04:12
@ADi From your code, if `second[i] != first[j]`, you still move `i` to `i + 1`. For example, `rulm` in `s2` and `rule` in `s1`, first three letters match and the fourth dismatches. And the index of `s2`, that is, `i`, still moves forward. So the fourth letter `m` in `s2` won't have chance to compare with first leter in `s1`. — longhua, Mar 19 '13 at 04:31
@ADi An advice: print out `i` and `j` after statement `j = current`. Check them carefully and you will see what is going on. — longhua, Mar 19 '13 at 04:34
This code was my own code. I tried to solve it but wasn't able to point out the problem. If yow want to degrade my reputation, at least give me a valid reason. If you (whoever did this)do this with new bees then there is no point in defining a factor as "reputation". I don't this there is anything wrong with this. Just coz you have higher reputation doesn't mean that you go on decreasing others. — Aditya Peshave, Mar 20 '13 at 20:02
@Code-Guru It seems I can't post any new question. What did I do wrong? Please explain. Thanks. — Aditya Peshave, Mar 23 '13 at 17:10

score 0 · Answer 1 · answered Mar 19 '13 at 04:27

I'm still not exactly sure how your algorithm works, but I think the value of j is getting too large too soon. I changed your else block to reset j to 0 each time, and the code now produces the desired output of "metallica".

           else 
           {   // if different char occured in between,
               // save current arraylist in one
               // and go forward
               one.add(sec);
               sec = new ArrayList<>();
               j = 0; // Start checking from the beginning of the string
               current = 0;
               break; // This will exit the while loop and move on to the next value of i
           }

anyhow I am setting current = 0 at the start of the for loop. So when this else loop executes j will be 0 anyhow. I think I am missing by 1 index somewhere. — Aditya Peshave, Mar 19 '13 at 04:33

score 0 · Accepted Answer · answered Mar 19 '13 at 04:41

I think that the problem is that your i gets incremented and compares the m from metallica of the second word with the last e of the first word, so next iteration it will start from the next letter which is the e (of the second word), that is why you are not getting the m at the beginning. Maybe if you use an i = i-1 every time you ended a common subsequence.

Find Longest common subsequence : my method

2 Answers2