10
\$\begingroup\$

I'm reading the book Head First Java and got an assignment to re-create the 99 Bottles of Beer song according to those lyrics found in the URL. I've had to do this with a Java while loop.

class SecondBeerSong {

    public static void main(String[] args) {
        int beerBottles = 99;
        String word;
        while(beerBottles >= 0) {
            word = "bottles";
            if(beerBottles != 0) {
                System.out.println(beerBottles + " " + word + " of beer on the wall, "+ beerBottles + " " + word + " of beer." );
                beerBottles--;

                if(beerBottles == 1) {
                    word = "bottle";
                }

                if(beerBottles > 0) {
                    System.out.println("Take one down and pass it around, " + beerBottles + " " + word +" of beer on the wall.");           
                } else {
                    System.out.println("Take one down and pass it around, no more " + word +" of beer on the wall.");           
                }
            } else {
                System.out.println("No more " + word + " of beer on the wall, no more " + word + " of beer.");
                System.out.println("Go to the store and buy some more, 99 bottles of beer on the wall.");
                break;
            }
            System.out.println();
        }

    }
}

My problems with the code:

  • Lack of readability
    • To many if statements for string building
  • It's confusing
  • Not "object-oriented"
  • The while loop gets ended with a break
\$\endgroup\$
  • 1
    \$\begingroup\$ Are you on Java 8? \$\endgroup\$ – h.j.k. Nov 22 '15 at 16:08
  • 1
    \$\begingroup\$ My local PC is, but the book is teaching me Java 7 if im not mistaken \$\endgroup\$ – Bas Nov 22 '15 at 16:10
  • \$\begingroup\$ One reason why I asked about Java 8 is to consider if there's any Java 8-related features to aid in the looping... ultimately, I decided to come up with my question here based on the concerns you raised, so maybe you can compare mine with yours. :) \$\endgroup\$ – h.j.k. Nov 23 '15 at 13:59
8
\$\begingroup\$

Move something outside of the loop

The main problem is that you are handling 0 and 1 bottles inside the loop, therefore making the whole loop more convoluted for one-shot conditions.

I suggest a loop to handle 99 to 2 and handling 1 and 0 manually:

for (int n = 99; n >= 2; n--) {
     System.out.println("Take one down and pass it around, " + n + " bottles of beer on the wall.");           
     System.out.println("Take one down and pass it around, " + (n - 1) + " more bottles of beer on the wall.");
}

System.out.println("1 and 0 bottles text, verbatim");

for loop

As you have seen above, I used a for loop as it is recommended over while when you already know how many times you'll loop as it groups together start, condition and step. Making the code shorter and more organized.

\$\endgroup\$
  • 1
    \$\begingroup\$ Why should I handle 1 and 0 manually though? I'd like the dynamic way though. Is it bad to handle conditions inside a loop? \$\endgroup\$ – Bas Nov 22 '15 at 18:52
  • 1
    \$\begingroup\$ @Bas you are already handling them manually, when you do if(beerBottles == 1) { word = "bottle"; } or if(beerBottles > 0) { you are making making changes to the predefined printing message. It is cleaner to do it after the loop. \$\endgroup\$ – Caridorc Nov 22 '15 at 18:54
  • \$\begingroup\$ Also outright better since you're not evaluating two conditions that are only met once each. \$\endgroup\$ – Legato Nov 22 '15 at 19:15
  • \$\begingroup\$ @Legato please be careful about micro-optimization, it is easy to fall into their trap. I moved that out of the loop only because it was cleaner, saving 0.0001 seconds did not influence me the littlest in making this decision. \$\endgroup\$ – Caridorc Nov 22 '15 at 19:18
  • 1
    \$\begingroup\$ An appreciated admonishment and definitely something to keep in mind. Previous comment more of an understanding issue noted due to asker's probable novicehood serving to point out a technical benefit in following your advice, however marginal. \$\endgroup\$ – Legato Nov 22 '15 at 19:49
0
\$\begingroup\$

There are two learning objectives to this programmers' etude: looping and pluralization. @Caridorc has already addressed looping: use a for-loop, and extract the termination case out of the loop.

You tried to do pluralization, but you have a bug:

2 bottles of beer on the wall, 2 bottles of beer.
Take one down and pass it around, 1 bottle of beer on the wall.

1 bottles of beer on the wall, 1 bottles of beer.
Take one down and pass it around, no more bottles of beer on the wall.

One approach to addressing the pluralization issue would be to define a function. However, I would like to point out that pluralization is a common problem that has a standard solution. Therefore, I recommend going with the standard solution rather than reinventing the wheel.

import java.text.MessageFormat;

public class ThirdBeerSong {
    private static final String VERSE_FMT =
        "{0,choice," +
          "0#No more bottles|" +
          "1#1 bottle|" +
          "1<{0} bottles} of beer on the wall, " +
        "{0,choice," +
          "0#no more bottles|" +
          "1#1 bottle|" +
          "1<{0} bottles} of beer.\n" +
        "{1,choice," +
          "0#Go to the store and buy some more|" +
          "0<Take one down and pass it around}, " +
        "{1,choice," +
          "0#99 bottles|" +
          "1#1 bottle|" +
          "1<{1} bottles} of beer on the wall.\n";

    public static void main(String[] args) {
        for (int b = 99; b > 1; b--) {
            System.out.println(MessageFormat.format(VERSE_FMT, b, b - 1));
        }
        System.out.print(MessageFormat.format(VERSE_FMT, 1, 0));
    }
}

If you find it acceptable to have an extra newline at the end of the output, you could get rid of the System.out.print() special case.

In the template, 0#99 bottles is admittedly an abusive hack. You can rework it if it bothers you.

\$\endgroup\$
  • \$\begingroup\$ I really have no idea what's happening here inside the VERSE_FMT string... \$\endgroup\$ – Bas Nov 24 '15 at 7:32
  • \$\begingroup\$ {0} and {1} are placeholders for the parameters b and b - 1, respectively. {0,choice,…} means to make a choice based on the first parameter (b). {0,choice,0#No more bottles|1#1 bottle|1<{0} bottles} means to make a choice based on b: if b is 0 then substitute "No more bottles", if b is 1 then substitute "1 bottle", and if b > 1 then substitute "b bottles". \$\endgroup\$ – 200_success Nov 24 '15 at 7:41
  • \$\begingroup\$ I'm going to save this answer for later. I think it's to complicated for me at this moment in my java learning. \$\endgroup\$ – Bas Nov 28 '15 at 17:33
-2
\$\begingroup\$

Beer Code:

Improved your code using ternary expressions.

class SecondBeerSong {

public static void main(String[] args) {

    String word;
    for (int beers = 10; beers > 0; beers--) {

        word = (beers == 1) ? " bottle" : " bottles";
        System.out.println(beers + word + " of beer on the wall, " + beers + word + " of beer.");

        word = ((beers == 1) ? " no more bottles" : (beers - 1) + (beers - 1 == 1 ? " bottle" : " bottles"));
        System.out.println("Take one down and pass it around," + word + " of beer on the wall.\n");

    }

    System.out.println("No more bottles of beer on the wall, no more bottles of beer.");
    System.out.println("Go to the store and buy some more, 99 bottles of beer on the wall.");

}
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Hi, try to elaborate on the way you improved the code. Don't post answers with only a series of changes. This might not help the OP understand why this is better code. \$\endgroup\$ – BCdotWEB Nov 23 '15 at 14:11
  • \$\begingroup\$ The first word excludes the number, whereas the second word incorporates the number. I find the inconsistency annoying. \$\endgroup\$ – 200_success Nov 24 '15 at 3:18

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.