12

Currently, I have a couple of singleton objects where I'm doing matching on regular expressions, and my Patterns are defined like so:

class Foobar {
  private final Pattern firstPattern =
    Pattern.compile("some regex");
  private final Pattern secondPattern =
    Pattern.compile("some other regex");
  // more Patterns, etc.
  private Foobar() {}
  public static Foobar create() { /* singleton stuff */ }
}

But I was told by someone the other day that this is bad style, and Patterns should always be defined at the class level, and look something like this instead:

class Foobar {
  private static final Pattern FIRST_PATTERN =
    Pattern.compile("some regex");
  private static final Pattern SECOND_PATTERN =
    Pattern.compile("some other regex");
  // more Patterns, etc.
  private Foobar() {}
  public static Foobar create() { /* singleton stuff */ }
}

The lifetime of this particular object isn't that long, and my main reason for using the first approach is because it doesn't make sense to me to hold on to the Patterns once the object gets GC'd.

Any suggestions / thoughts?

17

Java Pattern objects are thread safe and immutable (its the matchers that are not thread safe).

As such, there is no reason not to make them static if they are going to be used by each instance of the class (or again in another method in the class).

Making them instance variables, no matter how short (or long) their life time means that you are recompiling the regular expression each time you create an instance of a class.

One of the key reasons for this structure (Pattern being a factory for Matcher objects) is that compiling the regular expression into its finite automata is a moderately expensive action. However, one finds that often the same regular expression is used again and again in a given class (either through multiple invocations of the same method or different spots in the class).

The Matcher, on the other hand is rather light - it points to the state of the pattern within the Pattern and the location within the character array for the string.


For a singleton, it shouldn't matter too much, because after all, there's only one instance of it that sits around and you aren't recreating the singleton again and again (wait, 'lifetime of the singleton isn't that long'? Does this mean you are instantiating it multiple times over the course of the application?)

However, you will find that some static source code analyzers don't recognize that something is a singleton and will complain that you are creating instances of patterns from constants for each instance of the class.

The problem with all of this is that it is a not good (its not bad for a singleton either) choice and you may start ignoring other warnings for things that the compiler and analysis tools tell you about (read more about broken windows).

Related:

  • Awesome answer -- yes, i meant it's only ever created / used once, and once it goes out of scope it's done for good. Thanks for the follow up reading! – yamafontes Nov 2 '13 at 20:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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