-3
\$\begingroup\$

i am new on java and i have not more knowledge about java. please help me for optimizing it as much as possible

public void q1(String str, int[] arr) {
    String local = "findnumber";
    for(int i=0; i<arr.length; i++) {
        if(str.equals(local) && arr[i] * 2 > 10) {
            Integer in = new Integer(arr[i]);
            in = in * 2;
            System.out.print(in.toString());
        }
    }
}
\$\endgroup\$
4
  • 2
    \$\begingroup\$ Can u tell that what u r doing in this function so that picture may be more clear. \$\endgroup\$ Commented Jun 13, 2012 at 7:43
  • 1
    \$\begingroup\$ -1: I don't think this is a well formulated question, in the sense that you have not explained what you have tried, and why you need to optimize it. From the way it looks, you want someone else to do your dirty work, instead of trying to understand how to optimize code in a particular circumstance. And as @Joonas Pulakka has mentioned there is a sister-site called Code Review where you can ask such questions. \$\endgroup\$ Commented Jun 13, 2012 at 8:09
  • 2
    \$\begingroup\$ Unless very few numbers are printed, your main bottleneck will be writing to the console. (Can be 10-100x more than your cpu cost here) Given your output has no spaces between the numbers and doesn't appear to be useful, I would try to find a way to avoid do this at all (which would take no time and be the fastest) \$\endgroup\$ Commented Jun 13, 2012 at 8:33
  • \$\begingroup\$ Is this some sort of homework? \$\endgroup\$ Commented Jun 13, 2012 at 20:06

7 Answers 7

Reset to default
6
\$\begingroup\$

If your goal is to just print the numbers and you do not need them to be Integers you can use:

public void q1(String str, int[] arr) {
    if(!"findnumber".equals(str)) return;
    for(int i : arr) {
        if(i > 5) {
            System.out.print(i * 2);
        }
    }
}
\$\endgroup\$
4
\$\begingroup\$

First of all, you should first try to make your code readable and maintainable. That's the most important thing. Start by indenting it properly, and give meaningful names to your methods and variables.

Now to the performance, there are many things that can be optimized, but it won't change much, unless this method is called billions of times:

  • the local variable should be a constant
  • the str.equals(local) test should be executed once, out of the loop
  • you should not use Integer, but int: int in = arr[i] * 2;
  • the multiplication doesn't need to be computed twice

Here's a complete optimized version:

private static final String FIND_NUMBER = "findnumber";

public void q1(String str, int[] arr) {
    if (FIND_NUMBER.equals(str)) {
        for (int i = 0; i < arr.length; i++) {
            int doubleValue = arr[i] * 2;
            if (doubleValue > 10) {
                System.out.print(doubleValue);
            }
        }
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Shouldn't appending StringBuilder, and then dumping it to System.out be faster than calling System.out.print() so many times? \$\endgroup\$ Commented Jun 13, 2012 at 7:51
2
\$\begingroup\$

You can extract the check if str.equals(local) out of the loop. This will safe most of the operations performed in the loop:

public void q1(String str, int[] arr) {
    String local = "findnumber";
    boolean string_matches = str.equals(local);
    for(int i=0; i<arr.length; i++) {
        if(string_matches && arr[i] * 2 > 10) {
            Integer in = new Integer(arr[i]);
            in = in * 2;
            System.out.print(in.toString());
        }
    }
}

Or even you can return if it does not match:

public void q1(String str, int[] arr) {
    String local = "findnumber";
    if (!str.equals(local)) {
      return;
    }
    for(int i=0; i<arr.length; i++) {
        if(arr[i] * 2 > 10) {
            Integer in = new Integer(arr[i]);
            in = in * 2;
            System.out.print(in.toString());
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

Tip #1: Move the

if(str.equals(local))

up to line 3 (before for loop), so that, you can escape checking the same thing for each element of the array.

\$\endgroup\$
2
  • \$\begingroup\$ another optimization could be arr[i] > 5 instead of arr[i] * 2 > 10 \$\endgroup\$ Commented Jun 13, 2012 at 7:44
  • \$\begingroup\$ Insted of Integer in = new Integer(arr[i]); in = in * 2; use Integer in = arr[i]*2; (provided you are using java 1.5 and above. \$\endgroup\$ Commented Jun 13, 2012 at 7:47
1
\$\begingroup\$

You can move the str.equals(local) since if that's false, it will never run inside the loop:

public void q1(String str, int[] arr) {
    if(str.equals("findnumber")) {
        for(int i=0; i<arr.length; i++) {
            if(arr[i] > 5) {
                Integer in = new Integer(arr[i]);
                in = in * 2;
                System.out.print(in.toString());
            }
        }
    }
}

EDIT: and as sudmong suggested, you can do less math by dividing 10 by 2.

\$\endgroup\$
1
\$\begingroup\$

i) Instead of arr[i] * 2 > 10 you can use arr[i] > 5.

ii) Do the operation str.equals(local) outside the loop.

public void q1(String str, int[] arr) {
        String local = "findnumber";
        boolean isMatched = str.equals(local);
        for(int i=0; i<arr.length; i++) {
            if(isMatched && arr[i] > 5) {
                Integer in = new Integer(arr[i]);
                in = in * 2;
                System.out.print(in.toString());
            }
        }
    }
\$\endgroup\$
1
\$\begingroup\$

Try this:

 public void q1(String str, int[] arr)
 {
        if(str.equals("findnumber"))
        {
           String output = "";
           for(int i : arr)       
           {         
                if(i > 5)
                {
                   output += (i*2) + " ";
                }
           }
           System.out.println(output);
        }
  }

Print to the standard output at once, in cases where output is large it's quite good optimiaztion idea. Also you can use StringBuilder instead of String for large output.

\$\endgroup\$
0

You must log in to answer this question.