0

I am new here. I'm trying a C# problem on Leetcode about Longest Common Prefix. I know this problem has been solved MANY times. I'm just having a hard time understanding why my code doesn't work under certain conditions. When the input ["flower","flow","flight"] is put in, it works just fine and get the output "fl". However, when I get to the input ["ab", "a"], I suddenly get an IndexOutOfRangeException. Anyone know why? Here's my code:

public class Solution {
public string LongestCommonPrefix(string[] strs) {

    StringBuilder sb = new StringBuilder();
    int h = 1;
    int count = 0; 
    
    if (strs.Length == 1){
        return strs[0];
    }
    
    for (int i = 0; i<strs[0].Length; i++)
    {
         if ((strs[0])[i].Equals((strs[h])[i])){
            count++;
             if (h<strs.Length-1){
                 h++;
             }
         }  
    }      
    
     for (int j = 0; j < count; j++)
     {
         sb.Append((strs[0])[j]);
     }               
   return sb.ToString();       
 }
}
Blake
  • 1
  • 2
    You make dangerous assumptions with your "h" variable. – LarsTech Nov 29 '21 at 19:31
  • 1
    Your logic seems wrong to me. I think you need a nested loop of some kind, one loop to iterate over `strs` and one loop to iterate over the characters in each element within `strs`. Maybe I'm not understanding. – John Wu Nov 29 '21 at 19:38
  • What if the first string is the shortest? – Poul Bak Nov 29 '21 at 19:42
  • You're also assuming `strs` has at least one element. It could be empty. At the very least, I'd add an assert to check for that. – 3Dave Nov 29 '21 at 20:10
  • `sb.Append((strs[0])[j]);` You're looping through the first `count` characters in `strs[0]` and adding them as separate strings to `sb`. Is that really what you intended? Have a look at `String.Substring` https://learn.microsoft.com/en-us/dotnet/api/system.string.substring?view=net-6.0 – 3Dave Nov 29 '21 at 20:15

2 Answers2

2

You can debug by making a simple console app in Visual Studio. I added a Console.Writeline(strs[0] + strs[h]) to your code to demonstrate the logic as is. Running your code in debug, you can step through and see where the exception gets thrown:

Exception Location

Notice that the console prints the same thing each time, and that the strs[h] is only one character long. When you increment the i to 1, you're trying to access the second element of a one element array.

hallibut
  • 136
  • 1
  • 12
1

When the input ["flower","flow","flight"] is put in, it works just fine and get the output "fl". However, when I get to the input ["ab", "a"], I suddenly get an IndexOutOfRangeException. Anyone know why?

Because the algorithm as written loops until i has reached the length of the first string, not the shortest string. This is fine for flower/flow/flight because all these strings will find the common prefix "fl" before i comes close to running off the end of the shortest string; i is 2 by the time the algorithm determines that the o in flower is different to the i in flight and hence the code doesn't go out of bounds.

In the ab/a case, i is 1 and the algorithm hasn't yet determined the prefixes have diverged, you attempt to access the nonexistent index 1 on string "a" and get a crash

As comments have pointed out you also have a problem on your h logic; h should be cycled starting from 1 in a loop of its own inside the i loop. As it is now h just increments up to the number of strings-1 and stays there so in essence you end up only looking for prefixes in the first and last strings. If your strings were flower/flight/float/flowing/flowers you'd declare a prefix of "flower" because flight/float/flowing would only be considered when i was 0, 1 and 2 respectively

I suggest you write out the algorithm in comments, in the language you think in then have a go at turning it into c#

Caius Jard
  • 72,509
  • 5
  • 49
  • 80