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
– Hovercraft Full Of Eels, Commented Dec 17, 2018 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
– Jason Armstrong, Commented Dec 17, 2018 at 11:46

Jason Armstrong · Accepted Answer · 2018-12-17 11:52:00Z

2

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

answered Dec 17, 2018 at 11:52

Jason Armstrong

1,30014 silver badges20 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Glains · Accepted Answer · 2018-12-17 11:56:53Z

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, 2018 at 11:56

Glains

2,8833 gold badges19 silver badges32 bronze badges

8 Comments

abidinberkay Over a year ago

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 ?

Glains Over a year ago

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 Over a year ago

And yes, you have to create two Pattern objects if the regexes are different.

abidinberkay Over a year ago

i will use these regexes only once so do i need to make them static ? or just create private final ... .

Glains Over a year ago

What do you mean with you only use them once?

|

Rax · Accepted Answer · 2018-12-17 12:23:27Z

1

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.)

answered Dec 17, 2018 at 12:23

Rax

3751 gold badge7 silver badges18 bronze badges

2 Comments

abidinberkay Over a year ago

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 ?

Rax Over a year ago

Create them at class level.

Collectives™ on Stack Overflow

Creating global variable or creating instance inside of function

3 Answers 3

Comments

8 Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

8 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related