12 days of Christmas C++ using functions

Question

I am in a beginner C++ course and I am trying to create a program that outputs the 12 days of Christmas song using the two given function calls show_ordinal(int) and show_verse(int) as an assignment. It is supposed to call the verse and show the day with it's ordinal suffix and then loop the remaining verses depending on where the user decides to start from, so if it was show_verse(3) it would be on the 3rd day of Christmas... all the way to the pear tree. I started writing for the function to get the ordinal but kept getting segmentation fault errors.

#include <iostream>

using namespace std;

string show_ordinal(int);
void show_verse(int);

int main()
{
    cout << show_ordinal(2) << endl;

    return 0;
}

string show_ordinal(int x)
{
    switch (x % 10)
    {

        case 1:
            if (x % 10 == 1)
            {
                cout << x << "st";
            }
            break;

        case 2:
            if (x % 10 == 2)
            {
                cout << x << "nd";
            }
            break;

        case 3:
            if (x % 10 == 3)
            {
                cout << x << "rd";
            }
            break;

        case 4:
            if (x % 10 != 1 || x % 10 != 2 || x % 10 != 3)
            {
                cout << x << "th";
            }
            break;
    }

}

Testing the function by trying to call it with an int value of 2 in main,I have been working on it for awhile and cannot get it to work any help would be greatly appreciated.

Your code doesn't compile because you somehow copied line number with it. — nwp, Mar 07 '18 at 00:54
Unrelated warning. You use `string` without `#include `. This is unreliable. Sometimes it works. Sometimes it doesn't work. Don't count on other headers to include stuff you need. — user4581301, Mar 07 '18 at 00:57

score 1 · Answer 1 · answered Mar 07 '18 at 01:03

show_ordinal returns nothing for

cout << show_ordinal(2) << endl;

to print out. It promises to return a string, but it never does. This is bad form. When a function has a non-void return type it must return a result on all code paths or the function, and the program, is ill formed. A crash or other segfault is a common result, but you could get silent corruption of data, and that is much harder to track down.

Rather than couting all of your results, assign the results to a string and return the string.

score 0 · Answer 2 · answered Mar 07 '18 at 01:07

You never return a string from show_ordinal(), you just output to cout. Instead of using cout, I think you want to contsruct a string using x and your computed suffix and return that:

string show_ordinal(int x) {
    string out;

    switch (x % 10) {
        case 1:
            out = to_string(x) + "st";
            break;
        case 2:
            out = to_string(x) + "nd";
            break;
        case 3:
            out = to_string(x) + "rd";
            break;
        default:
            out = to_string(x) + "th";
            break;
    }

    return  out;
}

score 0 · Answer 3 · answered Mar 07 '18 at 01:19

You should probably count yourself lucky that you experienced a seg fault, because what you are doing is producing undefined behavior. If you don't believe me, check out what OnlineGDB does to your code.

The problem is that you define show_ordinal with a return value of std::string, but never return anything from the function. This produces undefined behavior in the C++ specification. To fix it you can do one of these two things:

Actually return a string. Rather than shifting into std::cout within the function, shift into an std::ostringstream instead, then return the stringified version:

#include<string>
#include<sstream>

std::string show_ordinal(int x) {
    std::ostringstream oss;
    switch (x % 10) {

    case 1:
        // Note:  I got rid of your extraneous "if" statement.
        // "switch" is already doing that work for you.
        oss << x << "st";
        break;

    /* more cases here */

    }

    return oss.str();
}

Define the function to return nothing. If you really want the function to handle the shifting to std::cout, define it with a void return signature and don't shift the output to std::cout in main:

#include<iostream>
#include<string>

void show_ordinal(int x);

int main() {
    show_ordinal(2);
}

void show_ordinal(int x) {
    switch (x % 10) {

    case 1:
        std::cout << x << "st\n";
        break;

    /* more cases here */

    }
}

Either of these should solve your problem.

Note: A few more things:

Please include all the headers for the standard libraries you are using. Add #include<string>.
You don't need those extra if statements in the case blocks. If your code made it to case 1, then x % 10 == 1 is guaranteed, so don't check it again.
show_ordinal won't do anything for x % 10 > 4. Consider replacing case 4 with default. See the documentation for the switch statement.
Please get out of the habit of using namespace std. It will get you in trouble in the future.

12 days of Christmas C++ using functions

3 Answers3