Simple String matching in java

Question

I am trying to implement simple String matching. The algorithm should return 1 if the source String contains the pattern String. I cannot understand why it is returning -1 on the following inputs

    String source = "aababba";
    String pattern = "abba";

Here is my implementation:

public static int findMatch(String source, String pattern)
{
    int j = 0,  pos = -1;
    boolean matched = false;

    if(source.length() < pattern.length())
        return -1;


    for(int i = 0; i < (source.length() - pattern.length()); i++)
    {
        if(source.charAt(i) == pattern.charAt(j))
            j++;
        else
            j = 0;
        if(j == pattern.length())
        {
            matched = true;
            break;
        }           
    }

    if(matched)
        return 1;       
    return -1;  
}

EDIT: As many of you suggested, the culprit was the for loop. I should have made it as follows. The rest of the code is the same. Other solutions are also possible as shown in the answers.

for(int i = 0; i <= (source.length() - pattern.length()); i++)
    {
        if(source.charAt(i+j) == pattern.charAt(j))
        {

Why you use (source.length() - pattern.length()) in for loop? — Maxim Shoustin
– Maxim Shoustin, Commented Jan 4, 2014 at 13:01
@MaximShoustin because it's no use checking the last three characters of the string if you are trying to match a 4-character pattern. It's a good idea, I just think the implementation is not exactly correct. — CompuChip
– CompuChip, Commented Jan 4, 2014 at 13:04
This isn't very clear on what your algorithm is intended to do. Why should it return 1 instead of -1? Is it because source contains the pattern? Or does source just have to contain all of the letters of pattern? Or something else? — Michael Yaworski
– Michael Yaworski, Commented Jan 4, 2014 at 13:27
To stick with the QA format of this site, you shouldn't edit answers into solutions. Either post your own answer and accept or accept an existing one and that's all. — Ross Drew
– Ross Drew, Commented Jan 4, 2014 at 13:44

Ross Drew · Accepted Answer · 2014-01-04 14:02:34Z

6

You are only checking three characters

source.length() - pattern.length()

So j will never be equal to pattern.length. You need to check the entire source.

for(int i = 0; i < source.length(); i++)
{
  if(source.charAt(i) == pattern.charAt(j))
  ...
}

Regarding the Solution

for(int i = 0; i <= (source.length() - pattern.length()); i++)
    {
        if(source.charAt(i+j) == pattern.charAt(j))
        {

Why have an additional subtraction and addition plus more complicated code when you can just step through the length of source?

edited Jan 4, 2014 at 14:02

answered Jan 4, 2014 at 13:05

Ross Drew

8,2563 gold badges44 silver badges53 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Callahan Over a year ago

The second characters will match when the pattern occurs. What do you mean?

Callahan Over a year ago

This will happen for the first characters. But when i = 4, j will be be 1 and hence b==b.

Ross Drew Over a year ago

Ah, I misunderstood the intent of the match. I've removed the irrelevant parts of my answer.

Ross Drew Over a year ago

...and extended on the relevant part :)

aga · Accepted Answer · 2014-01-04 13:06:50Z

2

The source of an issue is that you've limited the upper bound of i variable by (source.length() - pattern.length()). The for loop you've written can't check all the characters of source string, thus it'll return -1 even for some pairs of strings where it's possible to find a match.
Solution: rewrite the for loop like so:

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

answered Jan 4, 2014 at 13:06

aga

29.6k13 gold badges91 silver badges127 bronze badges

Comments

Callahan · Accepted Answer · 2014-01-04 13:04:53Z

1

Length of source is 7. Length of pattern is 4. Your statement i < (source.length() - pattern.length() will not be true for i > 2 so the loop just does not run "far" enough.

answered Jan 4, 2014 at 13:04

Callahan

4841 gold badge9 silver badges22 bronze badges

Comments

stinepike · Accepted Answer · 2014-01-04 13:06:13Z

1

You can easily use String.contains method to check if a string contains the patter.

answered Jan 4, 2014 at 13:06

stinepike

54.9k14 gold badges95 silver badges116 bronze badges

Comments

Alexia Wang · Accepted Answer · 2014-01-04 13:24:12Z

1

I think you already know the reason why it return wrong. You can try this:

j = 0；
while（j <= (source.length() - pattern.length())）{

    for (i = pattern.length(); i >= 0 && pattern[i] == source[i + j]; --i);

    if (i < 0)) {
        matched = true;
        break;
    }
    else j++；
}

about string matching, there are many methods, you can use KMP, BM and Sunday and so on, and performance comparation is: KMP < BM < Sunday, you can try these and they are very userful.

answered Jan 4, 2014 at 13:24

Alexia Wang

1312 silver badges12 bronze badges

Comments

cubitouch · Accepted Answer · 2014-01-04 13:08:58Z

0

By using this statement (source.length() - pattern.length()) you are looping on the string only on the three first chars of the source. So I will never find the entire pattern.

Try using pattern.length() instead, or better try compute the minimal value of both pattern.length() and source.length()

answered Jan 4, 2014 at 13:08

cubitouch

1,94715 silver badges28 bronze badges

2 Comments

Callahan Over a year ago

Aaannd the third or fourth answer of this kind. Dont read the other answers before posting?

cubitouch Over a year ago

Yes, you are right I didn't see them, my fault. But on the other hand I offered something else than you, by proposing to compute the shorter length instead of only source length.

Michael Yaworski · Accepted Answer · 2014-01-04 13:38:21Z

0

I'll assume it should return 1 because "aababba" contains "abba" at the end.

This, from your edit (solution) will return 1:

for(int i = 0; i <= (source.length() - pattern.length()); i++) {
    if(source.charAt(i+j) == pattern.charAt(j)) {

But from my assumption, you should be checking through every letter in source so I would use:

for(int i = 0; i < source.length(); i++) {
    if(source.charAt(i) == pattern.charAt(j)) {

Because why ignore the last bunch of characters?

But maybe I'm just assuming wrong what you want your algorithm to do because from what I've assumed, you could just use:

if (source.contains(pattern))
    return 1;
else
    return -1;

edited Jan 4, 2014 at 13:38

answered Jan 4, 2014 at 13:26

Michael Yaworski

13.5k19 gold badges72 silver badges98 bronze badges

3 Comments

Andromeda Over a year ago

Look at the charAt(i+j) inside the loop. That makes it similar to checking till source.length()

Michael Yaworski Over a year ago

@Andromeda Ah yes you're right - I missed that. Please remove your downvote. I edited my answer.

Michael Yaworski Over a year ago

@RossDrew Originally I had said that his edit would still give him the wrong output, but I was mistaken on that point.

Collectives™ on Stack Overflow

Simple String matching in java

7 Answers 7

4 Comments

Comments

Comments

Comments

Comments

2 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

7 Answers 7

4 Comments

Comments

Comments

Comments

Comments

2 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related