Optimizing String manupulation code

Question

I am trying to all possible child domains from a given host. I have written following code. It works but my only worry is performance.

Is it required to optimize this code further:

import java.util.Arrays;
import java.util.Collections;

public class DemoExtractHostArray {
    public static  String[] createHostArray(String host) {
        String[] stringArr = host.split("\\.");
        String[] hostArray = new String[stringArr.length];
        int hostIndex = 0;

        for(int index = stringArr.length-1; index>=0;index--){
            if(hostIndex==0){ 
                hostArray[hostIndex] = stringArr[index];
            }
            else{
                hostArray[hostIndex] = stringArr[index]+"."+hostArray[hostIndex-1];
            }
            hostIndex++;
        }
        Collections.reverse(Arrays.asList(hostArray));
        return hostArray;
    }

    public static void main(String a[]){
        for(String s: createHostArray("a.b.c.d.e.f")){
            System.out.println(s);
        }

    }
}

Output:

a.b.c.d.e.f
b.c.d.e.f
c.d.e.f
d.e.f
e.f
f

Well, does it perform badly? Did you profile your application and find out that this is a hotspot? — Kayaman
– Kayaman, Commented Jan 21, 2014 at 17:01
Optimization isn't "required" unless this is going to be used in a performance-critical loops somewhere. — keshlam
– keshlam, Commented Jan 21, 2014 at 17:02
Hint: never try to guess performance problems, it's a very inefficient way of finding them. — Kayaman
– Kayaman, Commented Jan 21, 2014 at 17:03
This code will get called millions of times. Based on the traffic. — Learn More
– Learn More, Commented Jan 21, 2014 at 17:15
If performance is really that much of an issue, you could always declare your methods as "native" and implement them using the Java Native Interface. — DeathByTensors
– DeathByTensors, Commented Jan 21, 2014 at 17:59

Sergey Kalinichenko · Accepted Answer · 2014-01-21 17:10:27Z

1

The only potential improvement to your code is removing this call:

Collections.reverse(Arrays.asList(hostArray));

Since you are creating the hostArray and then reversing it, you might as well change the loop to create the array in reverse order right away, so as to no longer requiring an explicit reversal:

// hostIndex is no longer required - remove the line below:
// int hostIndex = 0;
for(int index = stringArr.length-1 ; index>=0 ; index--){
    if(index == stringArr.length-1) {
        hostArray[index] = stringArr[index];
    }
    else{
        hostArray[index] = stringArr[index]+"."+hostArray[index+1];
    }
}

edited Jan 21, 2014 at 17:10

answered Jan 21, 2014 at 17:03

Sergey Kalinichenko

729k85 gold badges1.2k silver badges1.6k bronze badges

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

2 Comments

Michał Ziober Over a year ago

You can also create StringBuilder[] instead of String[]. Your source code in if statement could look like this: hostArray[index].append(....); Think about it.

Sergey Kalinichenko Over a year ago

@MichałZiober This will not improve much, because each concatenation is done only once, and all the intermediate results are kept.

75inchpianist · Accepted Answer · 2014-01-21 18:16:56Z

0

I would personally use recusion. This implementation does not require the reversal of the array and, in my opinion, may be easier to follow.

http://ideone.com/MnMZOL

package com.poachit.utility.web;

import java.util.Arrays;
import java.util.Collections;

public class DemoExtractHostArray {
     public static void createHostArray(String[] root, String[] result, int index) {

        String host="";
        int i = index;
        if (index == root.length) {
            return;
        }
        for ( ; i < root.length-1; i++) {           
            host += root[i] + ".";
        }

        if (i < root.length) {
            host += root[i];
        }

        result[index] = host;
        createHostArray(root, result, ++index);
    }

    public static void main (String[] args) throws java.lang.Exception
    {
        String host = "a.b.c.d.e.f";

        String [] tokens = host.split("\\.");

        String [] result = new String[tokens.length];

      createHostArray(tokens, result, 0);

      for (String s : result) {
        System.out.println(s);
      }

    }
}

edited Jan 21, 2014 at 18:16

answered Jan 21, 2014 at 17:28

75inchpianist

4,1021 gold badge24 silver badges42 bronze badges

3 Comments

keshlam Over a year ago

That's a tail-call. Recursion is unnecessary and overly expensive for this task; you can simply loop, incrementing index each time through the loop until index==root.length. Even simpler and easier to read, as well as more performant.

75inchpianist Over a year ago

unless he is doing hundreds of thousands of hosts performance really is negligible. You won't notice execution time hits from adding a few frames on the stack through recursion. this really is a matter of preference.

keshlam Over a year ago

Absolutely agree. But the question as posed was performance. And loop is clearer than tailcall for a beginner.

Manoj · Accepted Answer · 2014-07-27 08:06:58Z

You can optimize the code like this

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

public class Test {
    public static  String[] createHostArray(String host) {
        String[] stringArr = host.split("\\.");
        String[] hostArray = new String[stringArr.length];
        int hostIndex = 0;

        for(int index = stringArr.length-1; index>=0;index--){
            if(hostIndex==0){ 
                hostArray[hostIndex] = stringArr[index];
            }
            else{
                hostArray[hostIndex] = stringArr[index]+"."+hostArray[hostIndex-1];
            }
            hostIndex++;
        }
        Collections.reverse(Arrays.asList(hostArray));
        return hostArray;
    }

    public static  String[] betterCreateHostArray(String host) {
        List<String> hostList = new ArrayList<String>();
        do {
            if(!host.contains(".")) {
                hostList.add(host);
                break;
            } else {
                hostList.add(host);
                host = host.substring(host.indexOf('.')+1);
            }
        } while(host.length() > 0);

        return hostList.toArray(new String[hostList.size()]);
    }

    public static void main(String a[]){
        long startTime = System.nanoTime();
        String[] array = createHostArray("a.b.c.d.e.f");
        long endTime = System.nanoTime();
        long timeByFirstApproach = endTime - startTime;

        for(String s: array){
            System.out.println(s);
        }
        System.out.println("=====");
        startTime = System.nanoTime();
        array = betterCreateHostArray("a.b.c.d.e.f");
        endTime = System.nanoTime();
        long timeBySecondApproach = endTime - startTime;
        for(String s: array){
            System.out.println(s);
        }
        System.out.println(String.format("Time taken by first approach=[%d] nano seconds and\n"
                + "Time taken by second approach=[%d] nano seconds", timeByFirstApproach,timeBySecondApproach));
    }
}

and here the performance result

a.b.c.d.e.f
b.c.d.e.f
c.d.e.f
d.e.f
e.f
f
=====
a.b.c.d.e.f
b.c.d.e.f
c.d.e.f
d.e.f
e.f
f
Time taken by first approach=[1625572] nano seconds and Time taken by second approach=[308289] nano seconds
Second approach is more than 5 times faster than the approach you are following.

Collectives™ on Stack Overflow

Optimizing String manupulation code

3 Answers 3

2 Comments

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related