31
\$\begingroup\$

This is my first time writing JAVA, and I'm trying to start off on the right track.

Using my C++ knowledge, I was able to understand the syntax for the ternary operator, but I'm not sure how it should be approached properly in JAVA. I have used Ideone to test this, which include importss that apparently aren't even needed for this program, or so it seems.

I'd like a review over anything there is to offer, even if it involves telling me that starting with Ideone is not the best choice for whatever reasons.

import java.util.*;
import java.lang.*;
import java.io.*;

class BeginnerFizzBuzz {
    public static void main(String[] args) {
        for (int i = 1; i <= 100; ++i) {
            System.out.println(
                (i % 15 == 0) ? "FizzBuzz" :
                (i % 3 == 0) ? "Fizz" :
                (i % 5 == 0) ? "Buzz" :
                i
            );
        }
    }
}
\$\endgroup\$
9
  • 2
    \$\begingroup\$ While this is certainly valid code, this many nested ternaries generally make code harder to read and debug. Generally, I stick with 1 and sometimes if the expression is short, I'll do 2. \$\endgroup\$
    – mleyfman
    Commented Jul 11, 2014 at 0:38
  • 2
    \$\begingroup\$ @user26721: I was originally considering if/else, but I thought that would be redundant as well. But I can now see that this isn't too readable. \$\endgroup\$
    – Jamal
    Commented Jul 11, 2014 at 0:42
  • 5
    \$\begingroup\$ Couple of terminology things: (1) The language name is Java (upper/lower case). (2) The ?: operator is the conditional operator; a "ternary operator" is any operator that takes three operands (there is only one ternary operator in most programming languages, so people often say "ternary operator" when they mean "conditional operator"). \$\endgroup\$ Commented Jul 11, 2014 at 1:02
  • 3
    \$\begingroup\$ @GregHewgill it's a "CR thing" (see this meta post).. JAVA was intentional ;) \$\endgroup\$ Commented Jul 11, 2014 at 1:04
  • 1
    \$\begingroup\$ @Mat'sMug: I'm afraid I don't understand. On what basis are Java and JAVA treated as two different languages? As far as I can tell they follow the same rules as set out in the Java Language Specification. \$\endgroup\$ Commented Jul 11, 2014 at 1:07

3 Answers 3

15
\$\begingroup\$

even if it involves telling me that starting with Ideone is not the best choice for whatever reasons.

As a beginner, starting with Ideone is not the best choice IMO. For a number of reasons:

  • It is not a good practice to use import xxx.xxx.*. Only import the things that you actually need. In this case, you don't even need to import anything (because the things that you need are part of the java.lang package.
  • In IdeOne the filename is "Main.java" and there is no way to change that, which means that you will not learn that a public class in a file must have the same name as the file itself. I.e. the public class SomeClass must go in a file named SomeClass.java.
  • In IdeOne you are not able to declare a package name. A package name is mainly used to identify projects and the author of the code and so on. They are usually your domain-name backwards. For example, as I own zomis.net my package names are net.zomis.someproject. If you don't own a domain, well... make one up for now. com.jamal could work for you. Either way, usage of the "default"-package (i.e. not declaring a package at all) is discouraged.
  • In IdeOne there is no direct feedback for compiler errors. You have to click "Run" on your code to make sure that it has no compiler errors. In all modern Java IDEs you automatically get such feedback whenever you have finished writing a line of code (sometimes you have to hit Ctrl+S to save though before it will show the errors properly). This may not feel important at first, but once you get used to it you will really miss the feature in other IDEs.

As for the code itself, it isn't that bad. It is good that you removed IdeOne's throws Exception declaration.

Greg has already covered the aspects of the conditional operator, so there is only one thing I can possibly comment about, but this is just a personal opinion more than anything else and feel free to completely ignore this comment.

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

In Java, it is more common in such for-loops to use i++ rather than ++i. Although this has no effect whatsoever in this case, and I'm sure you're aware of the differences between i++ and ++i from your C++ (or is it ++C?) experience.

When using ++i or i++ alone (i.e. without an assignment to another variable), there is a myth that there is a performance difference in C++ and C, while apparently that is not the case.

In Java, it is also exactly the same (the same applies for C#, if you're wondering).


Of course, using i++ or ++i always matters when adding another assignment. If i = 1:

  • a = i++ will make a == 1
  • a = ++i will make a == 2
  • i == 2 after the statement in both cases
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I should've mentioned that Ideone was more of a "sneak peak" into Java. I never planned on using it further at this point, and I will install an IDE soon. \$\endgroup\$
    – Jamal
    Commented Jul 11, 2014 at 14:28
  • 1
    \$\begingroup\$ The pre/post increment stuff is a nice touch, but I'm mainly accepting this answer based on the advice regarding Ideone. I didn't know that things worked much differently amongst Java IDEs. I'm sure that will also be useful for others who find this question. \$\endgroup\$
    – Jamal
    Commented Jul 12, 2014 at 20:23
12
\$\begingroup\$

At first I wasn't sure whether this code would compile correctly (but obviously it does). The reason it does is due to two independent features of Java.

The System.out.println() method has an overload that takes an Object parameter (and then calls String.valueOf() to get a string representation of the object). Every type of object in Java derives from the Object base class. So, the literal string values you are passing are actually instances of the String class, which is assignable to Object.

But what about that i at the end of the chain of conditionals? That's not a string, and an int is not an Object either (primitive types in Java are not subclasses of Object). However, Java has a feature called "boxing" where primitive values can be placed inside an object container, in this case the Integer class. The compiler automatically boxes primitive values that are passed to Object parameters.

If you were to explicitly construct and cast everything your code is roughly equivalent to:

        System.out.println(
            (i % 15 == 0) ? (Object) new String("FizzBuzz") :
            (i % 3 == 0) ? (Object) new String("Fizz") :
            (i % 5 == 0) ? (Object) new String("Buzz") :
            (Object) new Integer(i)
        );

Because you are mixing different types in the conditional operator, you may find an unexpected behaviour here. For example, the following code does not compile:

        String s = (
            (i % 15 == 0) ? "FizzBuzz" :
            (i % 3 == 0) ? "Fizz" :
            (i % 5 == 0) ? "Buzz" :
            i
        );

The reason is that in a conditional expression a ? b : c, the types of b and c must match (or be subclasses of each other). In the above case, "Buzz" and i are different incompatible types. And in particular, i cannot be assigned to a String.

\$\endgroup\$
8
  • 1
    \$\begingroup\$ This would (mostly) apply to the ternary operator, right? If I were to have done this with separate println()s using ifs, would it have been proper? \$\endgroup\$
    – Jamal
    Commented Jul 11, 2014 at 0:56
  • \$\begingroup\$ Yes, that's correct. The compiler would choose a separate overload of println() for each type that you pass it. \$\endgroup\$ Commented Jul 11, 2014 at 0:57
  • 9
    \$\begingroup\$ Really small nit-pick: "...an overload that takes an Object parameter (and then calls the toString() method to get a string representation of the object)." This is not strictly accurate. It does not call the object.toString() method. It actually does String.valueOf(object), which has slightly different handling of null object values... \$\endgroup\$
    – rolfl
    Commented Jul 11, 2014 at 2:14
  • \$\begingroup\$ Sorry, I don't get this "review". The whole point of it, is that you don't understand the code posted, because you were not aware of some quite commonly known Java mechanisms? Even if they are not known, the code is readable and works without any surprises. So - what is the point of this review? (disclaimer: I'm asking seriously, but don't mean to be rude - sorry if this comment sounds too offensive - it's not my intention). \$\endgroup\$
    – BartoszKP
    Commented Jul 11, 2014 at 15:11
  • 1
    \$\begingroup\$ @BartoszKP: Code should be obviously correct. If I have to read it twice (and I have some experience with Java) because of two subtle interacting features of the language, then perhaps it could be written another way that is more obviously correct. That is what the point of my review was. \$\endgroup\$ Commented Jul 12, 2014 at 0:40
9
\$\begingroup\$

I realise that in CR we are encouraged to review the code rather than suggest alternatives but in this case there are good reviews so I hope I can be forgiven for posting alternative methods.

How about this?

enum FizzBuzz {

    Fizz(3),
    Buzz(5);
    final int divisor;

    FizzBuzz(int divisor) {
        // Each has a divisor.
        this.divisor = divisor;
    }

    public boolean useName(int n) {
        // Use the name if the divisor is a factor.
        return n % divisor == 0;
    }
}

public void test() {
    for (int i = 1; i < 20; i++) {
        StringBuilder s = new StringBuilder();
        // Walk all the FizzBuzzes
        for (FizzBuzz fb : FizzBuzz.values()) {
            if (fb.useName(i)) {
                // Add up all possible names.
                s.append(fb.name());
            }
        }
        if (s.length() == 0) {
            // No names used - use the number instead.
            s.append(i);
        }
        System.out.println(i + " - " + s);
    }
}

The sweet part of this method is that you can extend it trivially with say:

    Fizz(3),
    Buzz(5),
    Pling(7);

and it all still works in exactly the way you would expect.

This is essentially coding the algorithm and the data separately.

\$\endgroup\$
8
  • 1
    \$\begingroup\$ Alternate methods are okay, as long as they have some explanation behind it ("try this" answers don't work). You could explain this a bit further if you'd like, especially since I am a beginner. \$\endgroup\$
    – Jamal
    Commented Jul 11, 2014 at 14:40
  • 1
    \$\begingroup\$ I think so. The rest will involve individual questions, but the syntax looks simple enough. \$\endgroup\$
    – Jamal
    Commented Jul 11, 2014 at 14:44
  • 4
    \$\begingroup\$ It's a good start, but this is clearly missing dynamic class loading, reflection and a FizzBuzzManager class ;) \$\endgroup\$
    – Voo
    Commented Jul 11, 2014 at 23:31
  • 1
    \$\begingroup\$ An enum for FizzBuzz, that's a new one. I've made a slightly over-engineered solution with Strategy Pattern a year or so ago, but an enum? I like it. \$\endgroup\$ Commented Jul 13, 2014 at 13:16
  • 2
    \$\begingroup\$ @Voo - I'll try to fit a database in too in the next version. :D \$\endgroup\$ Commented Jul 13, 2014 at 15:43

Your Answer

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

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