5
\$\begingroup\$

I thought of writing a utility class where I can implement patterns that are useful for day-to-day parsing of strings, without having to to import java.util.regex everywhere. So I wrote this simple class as a model.

This was inspired by a (now deleted) Stack Overflow question:

For ex: String str="asd14sd67fgh007"; I want the output like: 1467007asdsdfgh

Right now I only added two patterns, with the intention of adding more as the need calls for it. I used for these, but other pattern-matching methods could [potentially] also be used, if the need calls for it.

I would be interested to know if there is a smarter way of doing this that I have not thought of, and in general, how can this be improved, to make adding more patterns and methods as easy as possible, keep the code clear and concise, and in general make it better for Mr. or Mrs. Maintainer.

Note: I wrote this using repl.it for the time being, and not actually added to a code base yet, hence the class not being declared as public. That will be rectified once it's added to a code base.

PatternMatcher class

import java.util.regex.*;

class PatternMatcher {

    public String inputString;

   /**
    * Constructor.
    */
    public PatternMatcher(String inputString) {
        this.inputString = inputString;
    }
    /**
     * Get only upper and lower case letters from inputString.
     * @return  String containing the upper and lower case letters from inputString
     */
    public String getLetters() {
        final Pattern LETTERS = Pattern.compile("[A-Za-z]");
        StringBuilder outputBuilder = new StringBuilder();
        Matcher letterMatcher = LETTERS.matcher(inputString);
        while (letterMatcher.find()) {
            outputBuilder.append(letterMatcher.group());
        }
        return outputBuilder.toString();
    }
    /**
     * Get only numbers/digits from inputString.
     * @return  String containing the numbers/digits from inputString
     */
    public String getNumbers() {
        final Pattern NUMBERS = Pattern.compile("[0-9]");
        StringBuilder outputBuilder = new StringBuilder();
        Matcher letterMatcher = NUMBERS.matcher(inputString);
        while (letterMatcher.find()) {
            outputBuilder.append(letterMatcher.group());
        }
        return outputBuilder.toString();
    }
}

Main class (for testing)

class Main {
      public static void main(String[] args) {
          String input = "asd14sd67fgh007";
          String expectedOutput = "1467007asdsdfgh";
          PatternMatcher inputMatcher = new PatternMatcher(input);
          StringBuilder actualOutputBuilder = new StringBuilder();
          actualOutputBuilder.append(inputMatcher.getNumbers());
          actualOutputBuilder.append(inputMatcher.getLetters());
          String actualOutput = actualOutputBuilder.toString();
          if (actualOutput.equals(expectedOutput)) {
              System.out.println("Pattern match succeeded");
          } else {
              System.out.println("Pattern match failed");
          }
      }
}
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$
final Pattern LETTERS = Pattern.compile("[A-Za-z]");

Every time either getLetters or getNumbers is called, a new regex is created and compiled.

This is not very efficient, considering regex compilation can be fairly slow. I would recommend moving the compiling to the constructor. Then, you can set the new Patterns to be fields of the class.

Now, the regexes will be compiled once and only once.


I don't quite understand your design.

  1. Initialize the class with a string.
  2. Use the class methods to get different parts of the string.

What if I want to check another string? Well, considering the fact that you made inputString public, I'm assuming you want the code to just set inputString to something else:

myPatternMatcher.inputString = 

Just to quickly say, the inputString should be private and there should be a setter for it. This is common Java practice.

However, that is not the point of this tip. My main point is that it is weird that I have to change a property of the class and then call another method of that class to work on the string I just passed in a second ago.

With setters, that would look like this:

myPatternMatcher.setInputString("foo");
myPatternMatcher.getLetters();

Isn't that a little weird having to call another method just to use the class?

I think a better approach would be to make the methods static. You could get rid of having to pass the inputString via the constructor, and you can move the pattern compilation to the fields themselves. Then, for each method, you can have it accept the string to check.

Now, your class would look like this:

public class PatternMatcher {
    public static String getLetters(String str) {
        ...
    }
    public static String getNumbers(String str) {
        ...
    }
}

Now, to use this class, it would look something like this:

PatternMatcher.getLetters("Foo");

This design makes more sense.

\$\endgroup\$
1
\$\begingroup\$

Here is how I ended up improving the code.

  1. I extracted the pattern declarations outside of the methods, so they would not be a need to create them anew each time a method was called. I also made them private and static.

  2. I removed the public String inputString; as well as the constructor. It has been pointed out that it wouldn't really make sense to have to instantiate this helper class into an object each time I need to use it.

  3. I made the methods static which makes the whole class much more practical to use.

Additionally, I made another method which combines two of the matcher methods, called getNumbersThenLetters().


import java.util.regex.*;

class PatternMatcher {
    private static final Pattern LETTERS = Pattern.compile("[A-Za-z]");
    private static final Pattern NUMBERS = Pattern.compile("[0-9]");
    /**
     * Get only upper and lower case letters from inputString.
     * @param String inputString  the String to be parsed according to the pattern matcher
     * @return  String containing the upper and lower case letters from inputString
     */
    public static String getLetters(String inputString) {
        StringBuilder outputBuilder = new StringBuilder();
        Matcher letterMatcher = LETTERS.matcher(inputString);
        while (letterMatcher.find()) {
            outputBuilder.append(letterMatcher.group());
        }
        return outputBuilder.toString();
    }
    /**
     * Get only numbers/digits from inputString.
     * @param String inputString  the String to be parsed according to the pattern matcher
     * @return  String containing the numbers/digits from inputString
     */
    public static String getNumbers(String inputString) {
        StringBuilder outputBuilder = new StringBuilder();
        Matcher numberMatcher = NUMBERS.matcher(inputString);
        while (numberMatcher.find()) {
            outputBuilder.append(numberMatcher.group());
        }
        return outputBuilder.toString();
    }
    /**
     * Get only the numbers/digits, then only the letters appended after the numbers.
     * @param String inputString  the String to be parsed according to the pattern matcher
     * @output String containing the numbers/digits, then the letters appended after the numbers, from inputString
     */
    public static String getNumbersThenLetters(String inputString) {
        StringBuilder outputBuilder = new StringBuilder();
        outputBuilder.append(PatternMatcher.getNumbers(inputString));
        outputBuilder.append(PatternMatcher.getLetters(inputString));
        return outputBuilder.toString();
    }
}

And in turn, it also made testing it quite a bit simpler:

class Main {
      public static void main(String[] args) {
          final String INPUT = "asd14sd67fgh007";
          final String EXPECTED_OUTPUT = "1467007asdsdfgh";

          StringBuilder actualOutputBuilder = new StringBuilder();
          actualOutputBuilder.append(PatternMatcher.getNumbersThenLetters(INPUT));
          String actualOutput = actualOutputBuilder.toString();

          if (actualOutput.equals(EXPECTED_OUTPUT)) {
              System.out.println("Pattern match succeeded");
          } else {
              System.out.println("Pattern match failed");
          }
      }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.