Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question

1 Answer 1

up vote 4 down vote accepted

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:

share|improve this answer
    
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! –  KepaniHaole Nov 2 '13 at 20:48

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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