Simple wildcard pattern matcher in Java

I have built a simple wild card pattern matcher algorithm.

I am concerned where it can be improved.

• Main concern is algorithm.

• Any other improvements are welcome too.

What it should does:

• Match * -> one or more of any character

• Match ? -> any single character similar to the file search wild card pattern (ex: Windows file search)

Code:

```
public class SimpleMatch {

private static enum State {

JUST_STARTED, NORMAL, EAGER, END
}

private final int pl; // pattern length
private final int pob; // pattern out bound
private final int sl; // string length
private final int sob; // string out bound
private final String p; // pattern
private final String s; // string to match

private static final char MATCH_ALL = '*';
private static final char MATCH_ONE = '?';

private int pp; // position of pattern
private int ps; // position of string
private State z; // state
private boolean m = false; // is match

public SimpleMatch(String p, String s) {

if (p == null || s == null) {
throw new IllegalArgumentException(
"Pattern and String must not be null");
}

this.p = p;
this.s = s;
pl = p.length();
sl = s.length();
if (pl == 0 || sl == 0) {
throw new IllegalArgumentException(
"Pattern and String must have at least one character");
}
pob = pl - 1;
sob = sl - 1;
pp = 0;
ps = 0;
z = State.JUST_STARTED;

}

private SimpleMatch(String p, String s, int pp, int ps) {

this(p, s);
this.pp = pp;
this.ps = ps;
}

private void calcState() {
//calculate state
if (z == State.END) {
return;
}

if (!psafe() || !ssafe()) {
z = State.END;
} else if (pc() == MATCH_ALL) {
if (!pnsafe()) {
z = State.END;

private final int pl; // pattern length
private final int pob; // pattern out bound
private final int sl; // string length
private final int sob; // string out bound
private final String p; // pattern
private final String s; // string to match

private int pp; // position of pattern
private int ps; // position of string
private State z; // state
private boolean m = false; // is match

Instead of writing a comment, why don't you use the complete name?

private final int length; // pattern length
private final int outbound; // pattern out bound
private final int stringLength; // string length
private final int stringOutBound; // string out bound
private final String pattern; // pattern
private final String matchString; // string to match

private int position; // position of pattern
private int stringPos; // position of string
private State state; // state
private boolean matchFound = false; // is match

Some examples, I used length instead of pattern because I think in this class pattern is obvious (or at least, I think.)

But why you have so much fields? Do you really need all them or we can convert them to variables?

And, if I don't remember wrong, the static final fields should go to the top of the class.

In your constructor, move the check conditions at top

public SimpleMatch(String p, String s) {

    if (p == null || s == null) {
        throw new IllegalArgumentException(
                "Pattern and String must not be null");
    }

    pl = p.length();
    sl = s.length();

    if (pl == 0 || sl == 0) {
        throw new IllegalArgumentException(
                "Pattern and String must have at least one character");
    }

    this.p = p;
    this.s = s;
    pob = pl - 1;
    sob = sl - 1;
    pp = 0;
    ps = 0;
}

And use better names! Even for params variables (who uses your class need to check the code / or documentation to understand what p and s are!)

I prefer to have all conditions in the top of the constructor so you (and who uses it) can see why the code thrown an exception without surfing between the constructor.

You can just do

private State z = State.JUST_STARTED; // state

and avoid

z = State.JUST_STARTED;

The same for others constructor, really what means pp, ps!

pl and sl are used inside constructor only, so convert to local variables:

int pl = p.length();
int sl = s.length();

You could use Guava Preconditions to improve the constructor style but whatever.

Method names, maybe:

mo -> matchOne
pc -> patternChar
sc -> stringChar
psafe -> hasNextPositionPattern (i really like the hasNext but it could be a bit strange like name anyway)
pnsafe -> hasNextPositionString (same)
ssafe -> checkStringPosition
ipp -> nextCharPattern
ips -> nextCharString

Now, you really need this methods?

nextCharString()
nextCharPattern()

Why not directly

stringPos++;
position++;

?

The same for other methods like patternChar.

SimpleMatch smo = new SimpleMatch(pattern, matchString, position + 1, stringPos + 1);

if (smo.match()) {
    state = State.END;
    matchFound = true;
    return;
}

Here you create another SimpleMatch instance, you can't alter the current instance?

Something like:

position++;
stringPos++;

if (match()) {
    state = State.END;
    matchFound = true;
    return;
}

Maybe you want to keep the original instance to the position because you want to return the match position?

What about

if (match()) {
    stringPos--;
    position--;

    state = State.END;
    matchFound = true;
    return;
}

But I don't see where you return there info... anyway, it still works (or at least, your test cases)

} else if (state == State.NORMAL) {
    if (matchOne()) {
        nextCharString();
        nextCharPattern();

        matchFound = true;
    } else {
        state = State.END;
        matchFound = false;
    }
}

The matchFound = false; is useless, it's already false. (you marked it false at method start matchFound = false;)

It's how it could look

```
public class SimpleMatch {

private static enum State {
JUST_STARTED, NORMAL, EAGER, END
}

private static final char MATCH_ALL = '*';
private static final char MATCH_ONE = '?';

private final int outbound;
private final int stringOutbound;
private final String pattern;
private final String matchString;

private int position;
private int stringPos;
private State state = State.JUST_STARTED;
private boolean matchFound = false;

public SimpleMatch(String pattern, String matchString) {
if (pattern == null || matchString == null) {
throw new IllegalArgumentException(
"Pattern and String must not be null");
}

int patternLength = pattern.length();
int stringLength = matchString.length();

if (patternLength == 0 || stringLength == 0) {
throw new IllegalArgumentExce

StackExchange Code Review Q#59007, answer score: 6