1

How can I optimize this code.

I want to reduce lines of code.

public class CoolDude {
    public static void main(String[] args) {
        for(int i = 100; i <= 500; ++i) {
            if(i%5 == 0 && i%11 == 0) {
                System.out.print("Cool Dude- ");
                System.out.print(i + "\n");
            } else if (i%5 == 0) {
                System.out.print("Cool - ");
                System.out.print(i + "\n");
            } else if (i%11 == 0) {
                System.out.print("Dude - ");
                System.out.print(i + "\n");
            }
        }
    }

}

Is there any way ?

Improve this question
4
  • 1
    You only need one System.out call per branch, and you can use println to avoid needing + "\n" every time Commented Feb 13, 2020 at 14:40
  • 1
    This is Java, so you could delete all of the newlines and have the whole thing on one line. But I don't think that's really what you wanted to ask. Commented Feb 13, 2020 at 14:40
  • Different language, but related: stackoverflow.com/questions/9461446/…. A note: in the original "FizzBuzz" interview question, you have a blank between "Dude" and "-", in the first test. Commented Feb 13, 2020 at 14:49
  • This is not a FizzBuzz question, but a very close one: there is no output for numbers not multiple of either divisors. It can be optimized in ways that FizzBuzz can't. Commented Feb 14, 2020 at 9:07

4 Answers 4

Reset to default
4

While Stephen M Irving's answer is pretty spot on and corrects all the beliefs found in your question, this still answers your question, trying to minimize the number of statements.

public class CoolDude {
  public static void main(String[] args) {
    for (int i = 100; i <= 500; i++)
      if (i % 5 == 0 || i % 11 == 0) // This is the condition where we decide to print something
        System.out.printf("%s%s- %d%n", i % 5 == 0 ? "Cool " : "", i % 11 == 0 ? "Dude " : "", i);
  }
}

However, this code duplicates one of the most expensive part: the modulo. Also, this solution is not readable !

When trying to figure solutions is useful to try several KPI and then find the best to optimize. In this case, you wanted to optimize the number of lines, it's definitely not the best as you can see above. If anything try first to get a working solution then a readable one and finally an optimized one where you document why it's optimized so that the readability is maintained.

Here, for instance, is the most optimized version I could come up with. It definitely contains more lines, but also is definitely faster, since I skip all invalid numbers and never do a modulo (only two divisions and two multiplications for the whole program).

public class CoolDude {
  public static void main(String[] args) {
    final int min = 100;
    final int max = 500;
    for (int i5 = nextMultiple(min, 5), i11 = nextMultiple(min, 11); i5 <= max || i11 <= max; ) {
      if (i5 < i11) {
        System.out.printf("Cool - %d%n", i5);
        i5 += 5;
      } else if (i11 < i5) {
        System.out.printf("Dude - %d%n", i11);
        i11 += 11;
      } else { // i5 == i11
        System.out.printf("Cool Dude - %d%n", i5);
        i5 += 5;
        i11 += 11;
      }
    }
  }
  static int nextMultiple(int number, int divisor) {
    int roundToLower = (number - 1) / divisor * divisor;
    return roundToLower + divisor;
  }
}
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

I was grappling with how to do this iteratively counting by 5's and 11's and 55's, but I had a hard time figuring out how to keep the numbers in the right order. Well done!
4

You could restructure your decision tree such that there will only ever have to be 2 checks (each with 1 operation and 1 comparison) against the number in the loop. Currently, your decision tree requires 2 operations and 2 comparisons in the best case (i is divisible by both 5 and 11) and 4 operations and 4 comparisons in the worst case (i is not divisible by either 5 or 11), but we can reduce that to always be just 2 comparisons and 2 operations, which will result in a more performant loop. In this way, i is only ever tested for divisibility against 5 and 11 one time for each number, so only 2 operations and 2 comparisons will need to be done no matter what the stage of the loop. This is the sort of optimization you should be looking at when trying to optimize a loop.

I also made your print method calls a printf call instead, reducing two print statements into 1. Here is a printf cheat sheet that you can use if you are unfamiliar with it.

Now, doing all this only reduced the size of your code by 1 line, and while I am sure that could be reduced further with some clever use of ternary operators or other methods, as a general rule, measuring code quality by number of lines is a terrible metric, and should never be used, particularly when we are talking about a compiled language like Java. There are a lot of things I could do to the code below that would reduce the line count at the expense of readability and/or performance, but there is no real point to that outside of competitions between programmers, like code golf (but even with that you are competing for the lowest character count, not line count).

Instead of shooting for shorter code, you should instead be striving for the best Big-O notation complexity so that your code is more performant, and fewer lines of code does not necessarily correlate with performance. 

public class CoolDude {
    public static void main(String[] args) {
        for (int i = 100; i <= 500; ++i) {
            if (i % 5 == 0) {
                if (i % 11 == 0) {
                    System.out.printf("Cool Dude - %d\n", i);
                } else {
                    System.out.printf("Cool - %d\n", i);
                }
            } else if (i % 11 == 0) {
                System.out.printf("Dude - %d\n", i);
            }
        }
    }
}
Improve this answer

2 Comments

If the goal is to be the fastest, a better solution would not iterate on i but on multiples of 5 and multiples of 11 and always check the lowest one and print accordingly, just like my second solution.
You are right. This was an easy and quick refactoring for a beginner to understand though. Usually on tasks like this they have you print out i as the default state when it isn't a multiple of anything you are checking for. My head was still stuck in that paradigm when I first started my answer. If that had been the case for this, you wouldnt be able to iterate on multiples. I wonder though, how much performance it takes to find the multiples and then check them against the max. That adds at minimum 2 and at most 3 additional operations to each loop before you even enter the actual logic.
0 
        IntStream.rangeClosed(100,500).forEach(i->{
        if(i%5 == 0 && i%11 == 0) {
            System.out.println("Cool Dude - "+i );
        } else if (i%5 == 0) {
            System.out.println("Cool - "+i );
        } else if (i%11 == 0) {
            System.out.println("Dude - "+i );
        }
    });
Improve this answer

3 Comments

How does using a stream here improve it at all?
IntStream do provide a little edge over a traditional for loop. Especially when your number range is above 3 digits, i wouldn't have bothered to refactor this if the number range was within 2 digits. This based on my production level code issue resolution experience. :) Cheers
Edge in what way? Speed? Readability? Please elaborate.
-1

The below should reduce lines of code, though it does not appear to run faster. It also corrects spacing around the hyphen and perhaps simplifies the logic.

public class CoolDude {
public static void main(String args[]) {
    for (int i = 100; i <= 500; ++i) {
        StringBuilder coolDude = new StringBuilder(15); //15 chars max "Cool Dude - 495"
        if (i % 5 == 0) {
            coolDude.append("Cool ".toCharArray());
        }
        if (i % 11 == 0) {
            coolDude.append("Dude ".toCharArray());
        }
        if (coolDude.length() > 0) {
            System.out.println(coolDude.append(("- " + i).toCharArray()));
        }
    }
}
}

REVISION: My point was that it was possible to take advantage of being able to do each mod calculation only once each time through the loop. That got lost in trying to save time with StringBuilders and a single line (which, as others pointed out, isn't a worthy goal). I clarified by using print and println instead.

public class CoolDude {
public static void main(String args[]) {
    boolean printed = false;
    for (int i = 100; i <= 500; ++i, printed = false) {
        if (i % 5 == 0) {
            System.out.print("Cool ");
            printed = true;
        }
        if (i % 11 == 0) {
            System.out.print("Dude ");
            printed = true;
        }
        if (printed) {
            System.out.println("- " + i);
        }
    }
}
}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.