Creating global variable or creating instance inside of function

Question

It is not a coding question but related to coding concept. I have a service class with some methods and also this class have following 2 private method for url parse and validation process.

private boolean isUrlFormatValid(String url) {
        Pattern pattern = Pattern.compile("^(https?:\\/\\/)?(www\\.)?([\\w]+\\.)+[\u200C\u200B\\w]{2,63}\\/?$");
        Matcher matcher = pattern.matcher(url);
        if (matcher.matches()) {
            return true;
        } else {
            LOG.error("Url format is not valid");
            return false;
        }
    }

    private String parseUrlDomain(String url) throws Exception {
        Pattern p = Pattern.compile("^(?:https?:\\/\\/)?(?:www\\.)?((?:[\\w]+\\.)+\\w+)");
        Matcher m = p.matcher(url);
        if (m.matches()) {
            System.out.println(m.group(1));
            return m.group(1);
        }
        throw new Exception("Url domain is not parsed ");
    }

These codes are working good but i am not sure about some points like:

1-As you see both method has common codes, creating pattern and matcher instances. Should i create instance of them at the beginning of class as global variable? If so what is the reason for it and what is the advantage of it.

2-In case of an error, i am not sure which one is better; throw exception as in second method or just log the error and continue as in first method.

Hence is there any best practise for it ? Thanks in advance.

If they don't change, why not declare them as constants, as `private static final String`? Or even an enum? — Hovercraft Full Of Eels, Dec 17 '18 at 11:45
Correction on terminology. Point 1 they are not global variables. They are instance variables if they are specific to an instance of an object. If they are shared across all instances of the class they would be static variables. — Jason Armstrong, Dec 17 '18 at 11:46

score 2 · Answer 1 · answered Dec 17 '18 at 11:52

Your Pattern instances are fine being instance or static. Marchers are not thread safe, so where you create them matters. If your class is running on a single thread then instance is fine. If you are using this class as a singleton, then create a new marcher in each method off of the shared Pattern. Have a look at this question

score 1 · Accepted Answer · answered Dec 17 '18 at 11:56

1

You actually discovered a good way to make your code faster. And that is also a common issue when suffering under slow regex execution. Since Pattern is thread safe, you can instantiate it one time reuse it in subsequent method calls.

private static final String REGEX = "some_regex";
private static final Pattern PATTERN = Pattern.compile(REGEX);

private boolean testRegex(String s) {
    return PATTERN.matcher(s).matches();
}

Please note that the Matcher class is not thread safe.

To your second point: Ask yourself the following question: Is the result of this operation important to the caller? If the anwer is yes, throw an exception of some kind to indicate what has gone wrong. If not, you can log the error to support later debugging.

answered Dec 17 '18 at 11:56

Glains

2,773
3
16
30

one point; i will use this regex string and pattern only in this class, so what is the reason for declaring it static ? and also regexs are different in two methods so do i need to declare 2 different pattern instance at the beginning of the class ? – abidinberkay Dec 17 '18 at 12:14
Have a look at the `static` keyword in Java: Static instance variables are shared between objects of the same class. You can have 1000 objects of your class, but still only one `Pattern` instance. Since its thread safe, it can be used in different threads. – Glains Dec 17 '18 at 12:35
And yes, you have to create two `Pattern` objects if the regexes are different. – Glains Dec 17 '18 at 12:35
i will use these regexes only once so do i need to make them static ? or just create private final ... . – abidinberkay Dec 17 '18 at 12:37
What do you mean with you only use them once? – Glains Dec 17 '18 at 12:38
i just use these patterns and regexes only once in their method. For example I have a function isUrlFormatValid() and i use pattern and regex in this method only. nowhere else. – abidinberkay Dec 17 '18 at 12:53
Yeah, but if you call this method multiple times, it already makes a difference, since the `Pattern` does not have to be created mutliple times. – Glains Dec 17 '18 at 13:13
But you class may be instanced multiplly,i.e., many objects . In this case you have many regexes. The `static` keyword can help here. – ZhaoGang Dec 17 '18 at 13:13

score 1 · Answer 3 · answered Dec 17 '18 at 12:23

Your pattern is based on a particular regex, it is not that you have only one regex. So, it is better you are getting the pattern as and when required. Again, coming to Matcher, it is based on your pattern. So you have created it as required. Making them a class level variable will make sense if you have only one regex for your URL to check against.
For errors- generally, you throw an exception (throws Exception - second case), when you want the caller API to take care of exception scenario. Suppose there are two callers of parseUrlDomain and these callers want to handle the URL parsing exception differently, then it more sense to have a throws clause. On the other hand, when you are very clear about how the error or exception cases are to be handled, you generally catch, log. (first case in your code snippet.)

i have 2 different regex, so is it logical to create patterns seperately in methods as in example ? or should i create 2 different pattern instance at the beginning of class ? — abidinberkay, Dec 17 '18 at 12:28

Creating global variable or creating instance inside of function

3 Answers3