Combinational string search using Java

Question

I wrote a program that receives 2 strings (Input and Search) in the Search string. The sign '+' indicates that if the substring before the '+' exists in the Input string. (The search string cannot start with '+' and there cannot be a '+' followed by another '+').

Can you review it for best coding practices and efficiency?

boolean notgood = false;
boolean break1 = false;
boolean break2 = false;
int counter = 0;

if (search.charAt(0)=='+'||search.charAt(0)=='*') {

    System.out.println("Invalid search striNG.");
    continue; 
} 
//////////////////////////////////////////////
for (i=0; i<search.length() && notgood==false; i++) {

    if (search.charAt(i)!='*' && search.charAt(i)!='+') {
        counter++;
    }
    if (counter == search.length()) {
        System.out.println("Invalid SEARCH string."); 
        notgood=true;
        break;
    }

    if ((search.charAt(i)=='*')||(search.charAt(i)=='+')) {

        if (i!=search.length()-1) {


            if ((search.charAt(i+1)=='*')||(search.charAt(i+1)=='+')) {

                System.out.println("INvalid search string."); 
                notgood=true;
                break1=true;
            }
        } 
    }
}
////////////////////////////////////////////
for (i=0; i<search.length() && !break1; i++) { 

    int c=0;

    if (search.charAt(i)=='+') {      

        String word = search.substring(0,i); 

        for (int j=0; j<input.length() && !break2; j++) { 

            if ((input.charAt(j) == word.charAt(c)) && c<word.length()) { 
                c++;
            }
            if (c>=word.length()) {

                System.out.println("Search string matches input string.");
                break1=true;
                break2=true;

            }
        } 
        if (c<word.length()) {
            System.out.println("Search string doesn't match input string.");
        }
    }

For example, for Input = 'abcd' and Search = 'ab+cd+', the result should be the strings match.

Answer 1

1.) Your search only looks for the first substring only (e.g. "ab"), it will never match the others given (e.g. "cd"). Why? Because You wrote:

String word = search.substring(0,i);

That would give us "ab+cd" for the second substring (maybe this is an error and you wanted to grab "cd"?) Anyway, for the first substring found you are setting break1 and therefore exiting the loop. But instead of setting break-variables, you can break directly by writing "break;". This would be faster and makes your code more readable.

2.) Always make tests (write JUnit-Tests) for your code. You can post them, too. In many companies you cannot check in your code without a corresponding test with full code coverage. If you have input="axb" and search="ab+", then it would match! Is this really intended? I think it's an error, and you would have found out if you wrote tests.

3.) Many times you wrote :

((search.charAt(i)=='*')||(search.charAt(i)=='+'))

This is a double DRY (Don't Repeat Yourself, clean code red grade) coding practice violation. Just evaluate search.charAt() once and store it in a variable "currentChar". Then evaluate the whole expression once and store it in a boolean variable "isDelimiter". Your code:

if (search.charAt(i)!='*' && search.charAt(i)!='+') {
    counter++;
}
...
if ((search.charAt(i)=='*')||(search.charAt(i)=='+')) {
...

would be reduced to:

char currentChar = search.charAt(i);
boolean isDelimiter = (currentChar=='*' || currentChar=='+');
if (!isDelimiter) {
    counter++;
}
...
if (isDelimiter) {
...

4.) Generally code is written once, but read on average about 10 times. That's why we have the coding practice to use meaningful names (clean code orange grade, source code conventions). So change "break1" to "isInvalidSeachString", "break2" to "hasNoMatch", "notgood" to "bad", "counter" to "validCharacterCounter" and "c" to "matchingCharacterCounter"

5.) Follow the Single level of abstraction principle.(clean code, orange grade) You are doing too much in a single method. You are verifying the correctness of the search string. Then you are tying to grab each substring and then match each substring. Meaningful names are much better than a meaningless separator like "///////...". Your structure should be following

public boolean isMatchingWholeSearch(String input, String search)
{
  boolean searchIsCorrect = hasNoStartingPlus(search) && hasNoDoublePlus(search);
  if (searchIsCorrect) searchForAllMatches(input, search);  
}
private boolean searchForAllMatches(String input, String search)
{
  for (int i=0, max=search.length(); i<max; i++) { // performance optimized for-loop
      boolean isSubstringSuffix = (search.charAt(i)=='+');
      if (isSubstringSuffix) {
          String substring = getSubstringBefore(search, i);
          boolean isFound = searchForSingleMatch(input, substring);
          if (isFound) return true;
      }
  }
  return false;
}
private boolean searchForSingleMatch(String input, String search)
{
  // here goes your inner loop. 
  ...
}
private boolean hasNoStartingPlus(String search)
{
   ...
}
private boolean hasNoDoublePlus(String search)
{
   ...
}

6.) For not reinventing the wheel, you could use java streams String.chars() with lambda expressions, or String.split() methods and regular expressions for verifying. That would make your code much more readable and very short. But maybe you can only use an old java version. Tell me if you can use a new one, and I'll give you code samples for each.