Performance Improvement in the code

Question

Problem Statement is-

In this case inclusion is null, and exclusion has some values, So I need to check whether temp is there in exclusion, if temp is there in exclusion then don't do anything or you can just get out of the loop, but suppose temp is not there in exclusion then call some method. I implemented the same feature below not sure whether we can improve this more or not. As it looks to me we can improve this more without using any boolean stuff I guess. As the stuff in main needs to be in a method and it is going to be called lot of times

public static void main(String[] args) {

    String temp = "77"; // It can also be 0
    String inclusion = null;
    String exclusion = "100;77;71";
    boolean bb = false;

    if(inclusion !=null) {
        System.out.println("Site Inclusion is not null");
    } else {
        for (String exc: exclusion.split(";")) {
            if(exc.equals(temp)) {
                bb =true;
                break;
            }
        }
        if(!bb) {
            // Call some method
        }
    }
}

Why are you using a String for exclusion as opposed to http://docs.oracle.com/javase/6/docs/api/java/util/HashSet.html? — Noel Yap, Jun 05 '12 at 18:59
You say that this will be called lots of times. What will be changing--`temp`, `exclusion`, or both? If `exclusion` doesn't vary, then this can be sped up a lot by preprocessing. — Ted Hopp, Jun 05 '12 at 19:07

score 1 · Answer 1 · edited May 23 '17 at 12:28

1

I'd suggest using StringTokenizer instead of String.split(). As discussed in this thread, it is quite a bit faster.

public static void main(String[] args) {

    String temp = "77"; // It can also be 0
    String inclusion = null;
    String exclusion = "100;77;71";
    boolean bb = false;

    if(inclusion !=null) {
        System.out.println("Site Inclusion is not null");
    } else {
        StringTokenizer st = new StringTokenizer(exclusion, ";");
        while (st.hasMoreTokens()) {
            if (temp.equals(st.nextToken())) {
                bb = true;
                break;
            }
        }
        if(!bb) {
            // Call some method
        }
    }
}

edited May 23 '17 at 12:28

Community

1
1

answered Jun 05 '12 at 19:01

Ted Hopp

222,293
47
371
489

Although in Java documentation appears like a legacy class, `StringTokeniker` is very fast. – Paul Vargas Jun 05 '12 at 19:06
So according to you boolean bb = true is necessary to achieve my problem scenario? – arsenal Jun 05 '12 at 19:06
1

@user1419563 - A flag like that is pretty common. There's nothing inefficient about it. (However, I'd move the declaration of `bb` inside the `else` branch and rename it to `matchFound` or something else meaningful.) – Ted Hopp Jun 05 '12 at 19:09
@PaulVargas - It is indeed an ancient class and the docs "discourage" its use (for readability reasons, it seems). However, it's not deprecated and according to the linked article in that other thread I referenced, can be twice as fast as String.split() (even for pre-compiled patterns). The real question is whether OP's `exclusion` string is a constant, in which case preprocessing of a different kind can avoid repeated string splitting and matching entirely. – Ted Hopp Jun 05 '12 at 19:16
temp, exclusion both can be changed anytime – arsenal Jun 05 '12 at 19:40
if I use regex here as exclusion format will be like this only `75;100;87;.....` so I can avoid for loop right? – arsenal Jun 05 '12 at 19:51
@user1419563 - If your main concern is performance, avoid regex (since you don't need it here). – Ted Hopp Jun 05 '12 at 20:31

score 1 · Answer 2 · answered Jun 05 '12 at 19:03

The most efficient implementation I can think of is something like

for(int i = exclusion.indexOf(temp);
    i != -1;
    i = exclusion.indexOf(temp, i + 1)) {
  // check if it's bracketed by ; or by the end of the string
  boolean goodStart = i == 0 || exclusion.charAt(i - 1) == ';';
  boolean goodEnd = i + temp.length() == exclusion.length()
    || exclusion.charAt(i + temp.length()) == ';';
  if (goodStart && goodEnd) {
    bb = true;
    break;
  }
}

which avoids the overhead of regexes, uses the built-in String.indexOf, and uses only constant extra variables.

+1 It is quite efficient. Approximately as fast as just a single indexOf. In the same timing test as mentioned in my answer, this took .07 seconds. — hatchet - done with SOverflow, Jun 06 '12 at 15:36

hatchet - done with SOverflow · Answer 3 · 2012-06-05T20:38:33.260

    exclusion = ";"+exclusion+";";
    temp = ";"+temp+";";

    bb = exclusion.indexOf(temp) >= 0;
    if(!bb) {
        // Call some method
    }

the first two lines only needed if you have no control of the format of exclusion and temp that's passed in. A quick test with many iterations of the code in your question took 1.74 seconds, and the same test with the code above took 0.38 seconds if exclusion and temp have the leading and trailing semicolon concatenated with every iteration of the timing loop. If they already have the leading and trailing semicolons, the time goes down to .07 seconds. For comparison, matches (regex) takes 1.18 seconds, and the StringTokenizer solution takes 0.50 seconds.

score 0 · Answer 4 · answered Jun 05 '12 at 19:12

0

bExists = Arrays.asList(exclusion.split(";")).contains(temp)

answered Jun 05 '12 at 19:12

srini.venigalla

4,945
1
15
29

1

There's no way that this has better performance than OP's original code. – Ted Hopp Jun 05 '12 at 19:19

score 0 · Answer 5 · answered Jun 06 '12 at 01:41

0

Searching a String by whatever means is O(N). Searching a HashSet is O(1).

Use a HashSet.

answered Jun 06 '12 at 01:41

user207421

289,834
37
266
440

Performance Improvement in the code

5 Answers5