1
\$\begingroup\$

This works in my Eclipse, but I think there is a better way to do it.

public static void main(String args[]) {

       for(int i = 1; i < 101; i++)
       {
           if(i%3 == 0)
           {
           System.out.println("repetitive of 3");
           }

           else if(i%5 == 0)
           {
           System.out.println("repetitive of 5");
           }

           else{
               System.out.println(i);
           }


       }


   }

}
\$\endgroup\$
  • \$\begingroup\$ Please see my notes on @UwePlonus answer for details of the edits. \$\endgroup\$ – rolfl Oct 2 '14 at 13:16
  • 2
    \$\begingroup\$ Technically 3 and 5 are not repetitions of 3 and 5. You should consider this case or use a different output. \$\endgroup\$ – wei2912 Oct 2 '14 at 13:55
  • 3
    \$\begingroup\$ Are you looking for multiples of 3 and 5? That's what your code does, at least. \$\endgroup\$ – Bergi Oct 2 '14 at 19:26
  • 8
    \$\begingroup\$ FizzBuzz! \$\endgroup\$ – blahdiblah Oct 2 '14 at 20:26
16
\$\begingroup\$

Moderation Note: This answer was being typed at the same time as the asker changed his code, to avoid this bug. Here's the code, as it was, when this answer was typed:

Program that prints out repetitives of 3,5 and both of them

public static void main(String args[]) {

       for(int i = 1; i < 101; i++)
       {
           if(i%3 == 0)
           {
           System.out.println("repetitive of 3");
           }

           else if(i%5 == 0)
           {
           System.out.println("repetitive of 5");
           }

           else if(i%5 == 0 && i%3 == 0)
           {
           System.out.println("repetitive of 3 and 5");
           }
           else{
               System.out.println(i);
           }
       }
   }
}

First of please format your code properly as it helps to read it. I've formatted the code as it is suggested for java:

public static void main(String args[]) {
    for (int i = 1; i < 101; i++) {
        if (i%3 == 0) {
            System.out.println("repetitive of 3");
        } else if (i%5 == 0) {
            System.out.println("repetitive of 5");
        } else if (i%5 == 0 && i%3 == 0) {
            System.out.println("repetitive of 3 and 5");
        } else {
            System.out.println(i);
        }
    }
}

Now for the review:

The third if will never be reached as the first if already matches.

So to get the result wanted you have to change the first and third if branch:

public static void main(String args[]) {
    for (int i = 1; i < 101; i++) {
        if (i%5 == 0 && i%3 == 0) {
            System.out.println("repetitive of 3 and 5");
        } else if (i%5 == 0) {
            System.out.println("repetitive of 5");
        } else if (i%3 == 0) {
            System.out.println("repetitive of 3");
        } else {
            System.out.println(i);
        }
    }
}

Using the method main() for small tests is fine but it is better to create objects and use them from the beginning.

Else the program look good so far.

\$\endgroup\$
  • 1
    \$\begingroup\$ Good answer. The OP managed to edit his code probably while you were writing your answer. That's not fair for you, but he's removed one of the key items in your review. I am going to add a notice to your answer. It would be a shame to lose it, and it's worth keeping. The OP scraped past the rules on this though.... \$\endgroup\$ – rolfl Oct 2 '14 at 13:10
  • 1
    \$\begingroup\$ I understand the frustration it causes when people change their code after asking their question. As the timing was slightly in the favour of the question asker, I have elected to resolve the situation this way. I appreciate that this is not great, and it is a compromise 'solution'. \$\endgroup\$ – rolfl Oct 2 '14 at 13:19
12
\$\begingroup\$

A couple of quick things.

  1. Be consistent with your white space. There's a good usage here:

     for(int i = 1; i < 101; i++)
    

    But not so much here:

     if(i%3 == 0)
    
  2. Let's back up to your loop declaration. I think it makes more sense to say less than or equal to 100 instead if less than 101.

    for(int i = 1; i <= 100; i++)
    
  3. Don't program in Main. Put your logic in a method and call it from Main.

\$\endgroup\$
11
\$\begingroup\$

Your indentation could use some work. I see you use Eclipse; This Stack Overflow answer tells you how to auto-format:

Ctrl + Shift + F

Or, in the main menu > Source > Format

It will take care of most spacing and indentation issues. You can even configure your style the way you want it (what you have now is not the standard Java dictates, see @Uwe Plonus's answer for an example).

The result:

public static void main(String args[]){

    for (int i = 1; i < 101; i++)
    {
        if (i % 3 == 0)
        {
            System.out.println("repetitive of 3");
        }

        else if (i % 5 == 0)
        {
            System.out.println("repetitive of 5");
        }

        else
        {
            System.out.println(i);
        }

    }

}

And if you look carefully, you'll see I snipped off a bracket. You had one too many.

\$\endgroup\$
  • 5
    \$\begingroup\$ Pssst... That's C# formatting. \$\endgroup\$ – RubberDuck Oct 2 '14 at 13:23
  • \$\begingroup\$ @RubberDuck I'm reusing his formatting style. \$\endgroup\$ – Pimgd Oct 2 '14 at 13:24
  • \$\begingroup\$ Ahhh fair enough. =) \$\endgroup\$ – RubberDuck Oct 2 '14 at 13:29
  • 2
    \$\begingroup\$ Every other company I've worked for required a different style, and each thinks that anyone who uses the other style is "One of THOSE guys". If you intend to program long-term I recommend learning to be comfortable with different styles, not calling one "Java" and one "C#" (Although saying the style recommended by Oracle or Microsoft would be completely accurate) \$\endgroup\$ – Bill K Oct 2 '14 at 16:36
  • 5
    \$\begingroup\$ @BillK the most important thing is consistency, but when an official style guide exists for C#, I think it's okay to call it "C# style". \$\endgroup\$ – RubberDuck Oct 2 '14 at 17:38
2
\$\begingroup\$

In Java and other similar languages inspired by C, there are two idiomatic ways to write a for-loop that repeats 100 times.

One way is

for (int i = 1; i <= 100; i++) {
    …
}

Use that when you want i to count from 1 to 100 (inclusive).

The other way is

for (int i = 0; i < 100; i++) {
    …
}

Use that when you want i to count from 0 to 99 (inclusive). You should also prefer this form if you only care about the total number of loops, and don't care about the values of i. (It's more "natural" since array indices are zero-based.)

Of those two forms, for this problem, you want the first.

However, don't mix metaphors, or you'll be likely to shoot yourself in the foot with an off-by-one error. The number 101 doesn't appear in the task that you are trying to solve, so it shouldn't appear in your code either.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.