CODEWITHSUNDEEP
javaregexif-statementoptimization
Infamous
Infamous

Reputation: 754

How to improve the code quality to see if a string matches either one of the regex's Java

In one of my projects I need to compare the URI with several regex patterns(15+ regex patterns). Currently I have used a if ladder to see if either one of them gets matched and there onward the logical part of the code is executed.

Glimpse of the code now:

if (uri.matches(Constants.GET_ALL_APIS_STORE_REGEX)) {    
    long lastUpdatedTime = InBoundETagManager.apisGet(null, null, tenantDomain, null);
    String eTag = ETagGenerator.getETag(lastUpdatedTime);
    if (eTag.equals(ifNoneMatch)) {
        message.getExchange().put("ETag", eTag);
        generate304NotModifiedResponse(message);
    }
    message.getExchange().put("ETag", eTag);
} 
else if (uri.matches(Constants.GET_API_FOR_ID_REGEX)) {   //        /apis/{apiId}
    apiId = UUIDList.get(0);
    String requestedTenantDomain = RestApiUtil.getRequestedTenantDomain(tenantDomain);
    long lastUpdatedTime = InBoundETagManager.apisApiIdGet(apiId, requestedTenantDomain, uri);
    String eTag = ETagGenerator.getETag(lastUpdatedTime);
    handleInterceptorResponse(message, ifNoneMatch, eTag);
} 
else if (uri.matches(Constants.GET_SWAGGER_FOR_API_ID_REGEX)) {   //  /apis/{apiId}/swagger
    apiId = UUIDList.get(0);
    long lastUpdatedTime = InBoundETagManager.apisApiIdSwaggerGet(apiId, tenantDomain); 
    String eTag = ETagGenerator.getETag(lastUpdatedTime);
    if (lastUpdatedTime == 0L) {
        log.info("No last updated time available for the desired API swagger json file");
    }
    handleInterceptorResponse(message, ifNoneMatch, eTag);
}

Can someone please introduce me with a more neat and clever way of doing this regex matching thing?

Upvotes: 0

Views: 97

Answers (2)

maaartinus
maaartinus

Reputation: 46482

Using multiple classes as @KonstantinLabun proposed is probably the way to go(*), but it shouldn't lead to much code duplication. So use an abstract class instead of (or in addition to an interface). Or (mis)use default methods.

abstract class URLHandler {
    abstract void handle();
    abstract Pattern urlPattern():
    final boolean isSupported(String url) {
        return urlPattern().matcher(url).matches();
    }
}

class GetAllApisStoreHandler extends URLHandler{
    private static final Pattern URL_PATTERN =
        Pattern.compile(Constants.GET_ALL_APIS_STORE_REGEX);

    Pattern urlPattern() {
         return URL_PATTERN;
    }

    public void handle(...) {
        ...
    }
}

There's no need to invent names for the PATTERN as its scope identified it already. The static field exists only as an optimization, so that the Pattern don't get compiled for each match.

(*) There's nothing wrong with a single class, as long as it's concise (I like spaghetti except in code) and doesn't leak implementation details. There's nothing wrong with multiple classes (except maybe on Android as 50 kB per class might matter) as long as they don't lead to code bloat. An enum is sometimes a good solution, too.

Explanation of abstract class vs. interface

An interface forces you to implement its methods(**), which may quickly lead to duplication. It's advantage is multiple inheritance and some conceptual purity.

An abstract class allows you to gather the common parts. But there's no dilemma, you can do both, see e.g., interface List and abstract class AbstractList.

(**) Since Java 8, an interface can have default methods, so this is no more true. Assuming you want to use them for this purpose. It can't declare any state, but it can access the state of the object. For example, my above URLHandler could be such an interface. There are still disadvantages, e.g., methods must be public and mustn't be final.

Upvotes: 1

Konstantin Labun
Konstantin Labun

Reputation: 3918

One url-type(regex) = one handler = one class. This way would be much easier to read and support especially if you have 15 regex checks.

interface URLHandler {
    void handle();
    boolean isSupported(String url);
}

class GetAllApisStoreHandler implements URLHandler{
    private static final Pattern GET_ALL_API_STORE_PATTERN = Pattern.compile(GET_ALL_APIS_STORE_REGEX);

    public boolean isSupported(String url) {
        return GET_ALL_API_STORE_PATTERN.matcher(url).matches();
    }

    public void handle(...) {
        ...
    }
}

class GetApiIdHandler implements URLHandler{
    private static final Pattern GET_API_ID_REGEX = Pattern.compile(GET_API_ID_REGEX);

    public boolean isSupported(String url) {
        return GET_API_ID_PATTERN.matcher(url).matches();
    }

    public void handle(...) {
        ...
    }
}

class GetApiIdHandler implements URLHandler{
    private static final Pattern GET_SWAGGER_FORAPI_ID_PATTERN = Pattern.compile(GET_SWAGGER_FOR_API_ID_REGEX);

    public boolean isSupported(String url) {
        return GET_SWAGGER_FORAPI_ID_PATTERN.matcher(url).matches();
    }

    public void handle(...) {
        ...
    }
}

class Main {
    private List<URLHandler> urlHandlers;

    public void method(){
        ...
        for (URLHandler handler : urlHandlers) {
            if(handler.isSupported(url)) {
               handler.handle(arg1, arg2, arg3, ...); 
            }
        }
        ...
    }
}

Upvotes: 2

Related Questions