I need help with creating a function!

[ZunLee]

Hey guys, I have written some code that I'd like to create as a function but I'm not getting any success.

This is the working code:

#include <iostream>
using namespace std;
#include <string>

int main()
{
int randomQuestion;
randomQuestion = rand() % 3 + 1;

string third, fifth, seventh;

switch (randomQuestion)
{



//New question
case 1:
cout << "What's the major third(3rd) of C?: ";
cin >> third;
while (third != "e" && third != "E")
{
cout << "wrong answer, please try again...";
cin >> third;
}
cout << "That's correct. Well done!\n\n";
return main();

//New question
case 2:
cout << "What's the major fifth(5th) of C?: ";
cin >> fifth;
while (fifth != "g" && fifth != "G")
{
cout << "wrong answer, please try again...";
cin >> fifth;
}
cout << "That's correct. Well done!\n\n";
return main();

//New question
case 3:
cout << "What's the major seventh(7th) of C?: ";
cin >> seventh;
while (seventh != "b" && seventh != "B")
{
cout << "wrong answer, please try again...";
cin >> seventh;
}
cout << "That's correct. Well done!\n\n";
return main();


}

}

And this code should give you an idea what I have in mind:

#include <iostream>
using namespace std;
#include <string>

int MajorIntervals(string third, string fifth, string seventh, char KeyOfX[]);
{
	int randomQuestion;
	randomQuestion = rand() % 3 + 1;


	switch (randomQuestion)
	{



		//New question
	case 1:
		cout << "What's the major third(3rd) of ";
		cin >> third;
		while (third != KeyOfX[0] && third != KeyOfX[1])
		{
			cout << "wrong answer, please try again...";
			cin >> third;
		}
		cout << "That's correct. Well done!\n\n";
		return MajorIntervals();

		//New question
	case 2:
		cout << "What's the major fifth(5th) of C?: ";
		cin >> fifth;
		while (fifth != KeyOfX[2] && fifth != KeyOfX[3])
		{
			cout << "wrong answer, please try again...";
			cin >> fifth;
		}
		cout << "That's correct. Well done!\n\n";
		return MajorIntervals();

		//New question
	case 3:
		cout << "What's the major seventh(7th) of C?: ";
		cin >> seventh;
		while (seventh != KeyOfX[4] && seventh != KeyOfX[5])
		{
			cout << "wrong answer, please try again...";
			cin >> seventh;
		}
		cout << "That's correct. Well done!\n\n";
		return MajorIntervals();


	}
}

int main()
{

	char KeyOfC[] = { 'E', 'e', 'G', 'g', 'B', 'b' };
	string third, fifth, seventh;


	MajorIntervals(third, fifth, seventh, KeyOfC);

}

I'd be forever greatful if any kind soul could help me!

[Ganado]

Line 5 of second code: You have a semi-colon at the end of the line. Remove it.

Other major difference: In first code, you are comparing strings to "string literals". This is fine.
In the second code, you are trying to compare strings to individual characters. This does not work, which may not be intuitive, but that's how it is.

In other words, this does not compile:

// Example program
#include <iostream>
#include <string>

int main()
{
    std::string a;
    char b; 
    a == b; // Error: no match for 'operator==' (operand types are 'std::string' and 'char')
}

You could fix this by making your char KeyOfC[] array actually be an array of strings, and using { "E", "e", "G", etc }, or by making third, fifth, and seventh be chars.

Also, while it doesn't really hurt, there's no point in passing third, fifth, and seventh as parameters, since their values are not used as input. Just declare them as local variables, or better yet, just have one local variable called "number" or "diatonic_number", etc.

[ZunLee]

Thanks man! It's starting to clear up now. This is the code I have now. Sometimes it work sometimes it doesn't. There are two problems left now; the function doesn't get the array value right.

This is the current code:

#include <iostream>
using namespace std;
#include <string>

int MajorIntervals(string third, string fifth, string seventh, string KeyOfX[])
{
	int randomQuestion;
	randomQuestion = rand() % 3 + 1;


	switch (randomQuestion)
	{



		//New question
	case 1:
		cout << "What's the major third(3rd) of ";
		cin >> third;
		while (third != KeyOfX[0] && third != KeyOfX[1])
		{
			cout << "wrong answer, please try again...";
			cin >> third;
		}
		cout << "That's correct. Well done!\n\n";
		return 0();

		//New question
	case 2:
		cout << "What's the major fifth(5th) of C?: ";
		cin >> fifth;
		while (fifth != KeyOfX[2] && fifth != KeyOfX[3])
		{
			cout << "wrong answer, please try again...";
			cin >> fifth;
		}
		cout << "That's correct. Well done!\n\n";
		return 0();

		//New question
	case 3:
		cout << "What's the major seventh(7th) of C?: ";
		cin >> seventh;
		while (seventh != KeyOfX[4] && seventh != KeyOfX[5])
		{
			cout << "wrong answer, please try again...";
			cin >> seventh;
		}
		cout << "That's correct. Well done!\n\n";
		return 0();


	}
}

int main()
{

	string KeyOfC[] = { "E", "e", "G", "g", "B" "b" };
	string third, fifth, seventh;


	MajorIntervals(third, fifth, seventh, KeyOfC);





}

When I start the program I always get case 3. And when I type in the value 'b' or 'B' it doesn't register as the "right" value for the while loop to stop.

The next problem I encounter is that I don't know what return value I should enter after the loop is done. When I had everything written in main I returned main(). But when the program is in the function I don't know what value I should return.

Thanks in advance!

[Handy Andy]

Hello ZunLee,

In addition to what Ganado has said.

Although you did not mention it your code leads me to believe that you are using some version of MSVS.

This is an idea of what you code could be.

#include <iostream>
#include <string>
#include <cstdlib>  // <--- For "rand()" and "srand()".
#include <ctime>    // <--- For "time()".

using namespace std;

int MajorIntervals()
{
    char KeyOfC[] = { 'E', 'e', 'G', 'g', 'B', 'b' };
    int randomQuestion{};
    string third, fifth, seventh;

    randomQuestion = rand() % 3 + 1;

    switch (randomQuestion)
    {
        //New question
        case 1:
            cout << "What's the major third(3rd) of ";  // <--- Question does not match format of case 2 and case 3.
            std::getline(std::cin, third);  // <--- Changed.

            while (third[0] != KeyOfC[0] && third[0] != KeyOfC[1])
            {
                cout << "\n    wrong answer, please try again...:\n\n";  // <--- Changed.

                cout << "What's the major third(3rd) of ";  // <--- Added.
                std::getline(std::cin, third);
            }

            cout << "\n  That's correct. Well done!\n\n";

            return MajorIntervals();  // <--- Recursive function call with no way out.

In VS "iostream" will eventually include "cstdlib" along with some others. DO NOT COUNT on the being the norm. Not all compilers have their header files set up the same as VS.

It is better to include what you need and let the header guards keep the code from being included more than once.

I do not believe it makes any difference, but the line using namespace std; usually follows the include files. It is not needed for any of the standard include files, but for the code that follows. It is also best not to use this line, but that is a different topic in its-self.

To reiterate what Ganado said. The colon at the end of line 5, in your second code, or line 8, in my code, makes a very nice prototype which you do not need because the function is compiled before "main".

Had the function been defined after "main" you would need the prototype so the compiler would know what to expect.

I removed all the parameters of the function. These variables are only used in the function and are better defined there unless you have some plan on using them later in which case the strings should be passed by reference, so that any changes made in the function would be reflected back in "main", but you are not showing that at this time.

It is also a good idea to initialize your variables when defined so they do not contain a garbage value. The exception is "std::string"s, "std::vector"s and other containers which have a zero size when defined and do not need initialized.

The line choosing a random number is OK, but does have its limitations.

In "case 1" the comment on the prompt covers that.

Using "std::getline" is a better choice as you have defined your variables as "std::string"s. This will leave the input buffer empty when finished whereas the formatted input of cin >> third; could leave something in the input buffer causing the next "cin" to not wait for input, but use what is in the buffer. This will cause the while loop to continue until the input buffer is emptied, so you could see several error messages when you do not need to.

The while condition is an alternative to changing the strings to "char". By accessing just one element of the string you are comparing a "char" to a "char". Also it will not matter how long the string is because you are only comparing the first element of the string.

The way you have it set up it is best to re-prompt while inside the while loop.

They are little things, but look at the way I used spaces to indent the error message and the correct response compared to the prompts.

Also there are several places in your code where you have extra blank lines that are not needed and other places where you do. It is a small bit of code, but notice where I used blank lines to break up the code. This makes it easier to read and follow.

The last line of the case statement is a recursive call to the function. This may do what you want, but there is no way out. Also the function promises to return an "int" but it does not. You are trying to return the return value of the function, but the function never returns any "int" variable or value. This tends to create an endless loop.

What you see here for "case 1" can be applied to "case 2" and "case 3".

In "main" other than removing the variable definitions I added:
srand(static_cast<unsigned int>(time(nullptr)));. First if you are going to use "rand()" you need to seed the RNG before calling "rand". Otherwise it will seed the RNG with 1 making sure that all your calls to "rand" produce the same random numbers in the same order. This line of code may seem a bit much, but it is the best way to write it.

If you are going to use "rand()" this video is worth watching.
https://channel9.msdn.com/Events/GoingNative/2013/rand-Considered-Harmful

And this may be worth the read.
https://web.archive.org/web/20180123103235/http://cpp.indi.frih.net/blog/2014/12/the-bell-has-tolled-for-rand/

Andy

[dutch]

BTW, there's no such thing as a "major 5th". You mean "perfect 5th". :-)

[Handy Andy]

Hello ZunLee,

Some additions I made that may help.

Above your "MajorIntervals()" function I added:

int ContinueLoop()
{
    int choice{};
    char input{};

    std::cout << "\nWould you like another question? (y/n): ";
    std::cin >> input;

    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');  // <--- Requires header file <limits>.

    if (std::toupper(input) == 'Y')  // <--- Requires header file <cctype>.
    {
        return 1;
    }

    return 0;
}

The functions "std::tolower()" and "std::toupper()" are from header file "cctype". Or you could use if (input == 'C' || input == 'c'). In this case I used "1" to mean true the program should continue and zero to mean false.

In the "MajorIntervals()" function replace the return statements with "break;" and after the closing brace of the switch put return ContinueLoop();.

The name of the function is your choice to change if you want.

In "main" I used this:

do
{
    cont = MajorIntervals();

} while (cont);

The while condition makes use of the zero or 1 that is returned.

Andy