Printing lyrics of 99 Bottles of Beer

Question

I wrote a C++ program for 99 Bottles of Beer, but it's kind of messy. Is there any way I can make it short and cleaner and more easier to read?

/*
 * Write a program that prints out the entire lyrics to a full rendition of "99 bottles of beer"
 */

#include <iostream>

using namespace std;

int main (void)
{
    int bottle = 99;
    while (bottle > 0)
    {
        cout << bottle << " bottles of beer on the wall, "
             << bottle << " bottles of beer." << endl;

        bottle--;

        cout << "Take one down and pass it around, " << bottle << " "
             << "bottles of beer on the wall.\n" << endl;

        if (bottle == 1)
        {
            break;
        }

    }

    cout << "1 bottle of beer on the wall, 1 bottle of beer." << endl
         << "Take one down and pass it around, no more bottles of beer on the wall.\n" << endl
         << "No more bottles of beer on the wall, no more bottles of beer." << endl
         << "Go to the store and buy some more, 99 bottles of beer on the wall.";

    return 0;
}

Answer 1

A few short comments:

Don't use namespace std:

It's bad for a few reasons, but in short: You can have big name conflicts if you are using namespace std;. For more information: Why is using namespaec std considered bad practice?

Break conditions:

well lookie here:

while (bottle > 0) 
{
    //print the lyrics
    if (bottle == 1) 
    {
         break;
    }
}

This could be much simpler:

while (bottle > 1)
{
     //print the lyrics
}

Naming:

bottle is a crap name for a counter. Make it bottleCount, leftBottles or about anything else, but don't name it bottle. Because it isn't a bottle...

Answer 2

In addition to the comments in the other answers I'd recommend you to create a function to print the sentences on the screen.

Something like

while (bottles > 0)
{
    printLyrics(bottles);
    bottles--;
}

In this case you need to handle the special case when you have a single bottle left.

void printLyrics (int bottles)
{
    if (bottles == 1)
    {
        printLastLyrics ();
    }
    else
    {
        printOtherLyrics (bottles);
    }
}

I think this way the problem is better decomposed and the special case is shown at the right level of abstraction.

Answer 3

This seems like a place that template meta programming would be useful (after all, we don't want to waste precious run-time in a program that's likely to execute this often).

#include <iostream>

template<int N>
struct song {
    inline static void sing() {
        std::cout << N << " bottles of beer on the wall, "
            << N << " bottles of beer.\nTake one down, pass it around, "
            << N - 1 << " bottles of beer on the wall.\n\n";
        song<N - 1>::sing();
    }
};

template<>
struct song<1> {
    inline static void sing() {
        std::cout << "1 bottle of beer on the wall, 1 bottle of beer.\n"
            << "Take it down and pass it around, no more bottles of beer on the wall.\n\n"
            << "No more bottles of beer on the wall, no more bottles of beer.\n"
            << "Go to the store and buy some more, 99 bottles of beer on the wall.\n";
    }
};

template<>
struct song<2> {
    inline static void sing() {
        std::cout << "2 bottles of beer on the wall, "
            << "2 bottles of beer.\nTake one down, pass it around, "
            << "1 bottle of beer on the wall.\n\n";
        song<1>::sing();
    }
};

int main() {
    song<99>::sing();
    return 0;
}

Although the ordering of the templates (first the general template, then the specialization for 1, and finally the specialization for 2) may seem strange and haphazard, it's actually necessary. The general case must come before any specialization. The specialization for 2 uses the specialization for 1, so it must follow it. Otherwise, the specialization for 2 would attempt to use the general template for 1, and only after it did so would the template for 1 be found, rendering that invalid.

Unfortunately, at least as I've written the code, this doesn't reduce the code size. OTOH, I believe it at least gets the grammar correct.

Answer 4

Not needed code :

You can actually remove this :

if (bottle == 1)
{
    break;
}

You are in a while loop so your loop will continue as long bottle is bigger then 0.
So I'm guessing you inserted that code for the last bottle.
You just have to change :

while (bottle > 0)

to

while (bottle > 1)

for the same result.

Naming :

While you have not many var's to call => I like that you already point to the bottle.
Still I'm thinking bottleCount could be a better var name.

While loop :

Well it doesn't make a big difference but for your own sake, use a for-loop.
You don't forget to substract one then.

for (int bottleCount=99; bottleCount > 1; bottleCount--)

Summary:

While I'm no c++ coder, it looks for the rest good.
If you really want to score high you could use recursion for the for-loop.

Answer 5

Output errors

You have a pluralization error:

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

Also, it is conventional to end the output of a program with a newline. Yours ends with just a period.

Problem structure and decomposition

You have a special case for the one-bottle verse. Perhaps that could be generalized a bit.

As illustrated above, the primary challenge is to get the pluralization and capitalization exactly right. One possible strategy would be to treat this as an internationalization problem.

However, it would be more interesting and instructive to approach it as a C++ programming exercise. Therefore, I think that an object that knows how to print itself with the right pluralization and capitalization would be a good idea.

Suggested solution

#include <iostream>

class BeerWall {
  public:
    BeerWall(unsigned n=99) : n(n), capitalizeNext(false) {}

    BeerWall &consume(int bottles=1) {
        n -= bottles;
        return *this;
    }

    BeerWall &buy(int bottles=1) {
        // Not to be confused with barfing it up.
        return consume(-bottles);
    }

    BeerWall &capitalize(bool cap=true) {
        capitalizeNext = cap;
        return *this;
    }

    unsigned remaining() const {
        return n;
    }

    friend std::ostream &operator<<(std::ostream &os, BeerWall& b) {
        switch (b.n) {
          case 0:
            os << (b.capitalizeNext ? "No more bottles" : "no more bottles");
            break;
          case 1:
            os << "1 bottle";
            break;
          default:
            os << b.n << " bottles";
        }
        b.capitalizeNext = false;
        return os;
    }

  private:
    unsigned n;
    bool capitalizeNext;
};

int main(void)
{
    BeerWall bw;
    do
    {
        std::cout << bw.capitalize() << " of beer on the wall, "
                  << bw << " of beer.\n";

        if (!bw.remaining()) break;

        std::cout << "Take one down and pass it around, "
                  << bw.consume() << " of beer on the wall.\n" << std::endl;
    } while (true);

    std::cout << "Go to the store and buy some more, "
              << bw.buy(99) << " of beer on the wall." << std::endl;
}

Answer 6

Chillworld alluded to this solution, I will expand a little bit.

your code:

int main (void)
{
    int bottle = 99;
    while (bottle > 0)
    {
        cout << bottle << " bottles of beer on the wall, "
             << bottle << " bottles of beer." << endl;

        bottle--;

        cout << "Take one down and pass it around, " << bottle << " "
             << "bottles of beer on the wall.\n" << endl;

        if (bottle == 1)
        {
            break;
        }
    }

    cout << "1 bottle of beer on the wall, 1 bottle of beer." << endl
         << "Take one down and pass it around, no more bottles of beer on the wall.\n" << endl
         << "No more bottles of beer on the wall, no more bottles of beer." << endl
         << "Go to the store and buy some more, 99 bottles of beer on the wall.";
    return 0;
} 

Everyone talked about the while statement not being set up efficiently and the if block not being needed.

if you set up the while statement to say while (bottle > 1) and remove the if block of code it would look like this.

int main (void)
{
    int bottle = 99;
    while (bottleCount > 1)
    {
        std::cout << bottleCount << " bottles of beer on the wall, "
             << bottleCount << " bottles of beer." << std::endl;

        bottleCount--;

        std::cout << "Take one down and pass it around, " << bottleCount << " "
             << "bottles of beer on the wall.\n" << std::endl;
    }

    std::cout << "1 bottle of beer on the wall, 1 bottle of beer." << std::endl
         << "Take one down and pass it around, no more bottles of beer on the wall.\n" << std::endl
         << "No more bottles of beer on the wall, no more bottles of beer." << std::endl
         << "Go to the store and buy some more, 99 bottles of beer on the wall.\n";
    return 0;
} 

unfortunately you still have the issue of the plural form of bottle when you get down to one bottle left, I would assume though after 98 bottles of beer it would take a miracle of science to even sing the last verse.