Review my permutations and combinations program

[Satan]

Hey guys do you see any possible improvements in this code? Any other suggestions are welcomed as well, I hope I'm not trailing any bad habits. For example I have used system("cls") to clear the screen.

The program is made for the purpose of generating permutations and combinations problems and letting the user solve them (and check whether the input is valid, incorrect, log incorrect and invalid attempts and the time taken to get it right).

/* TO DO LIST:

*/

#include <iostream>
#include <ctime>
#include <Windows.h>
#include <vector>
#include <string>
#include <algorithm>
/* for transform() */

int random(int n, int m) {
	return rand() % (m - n + 1) + n;
}

unsigned int factorial(unsigned int input) {
	unsigned int factorial = 1;
	for (unsigned int i = 1; i <= input; ++i) 
		factorial *= i;
	return factorial;
}

int main() {
	srand((unsigned)time(NULL));
	std::clock_t start;

	bool validator = false;
	int inputted_n;
	int inputted_r;
	int no_of_questions;

	validator = true;
	while (validator) {
		system("cls");
		std::cout << "How many questions would you like to answer?: ";
		std::cin >> no_of_questions;
		if (!std::cin) {
			std::cin.clear();
			std::cin.ignore();
		} 
		else if (no_of_questions == 0) {
			std::cout << "Great, you're done answering 0 questions! What now?";
			Sleep(1000);
			std::cin.get();
			std::cin.get();
		} 
		else if (no_of_questions < 0) {
			std::cout << "Hmm, how do you think you're gonna get around answering that many questions?";
			Sleep(1000);
			std::cin.get();
			std::cin.get();
		}
		else
			validator = false;
	}

	validator = true;
	while (validator) {
		system("cls");
		std::cout << "Range for 'n': ";
		std::cin >> inputted_n;
		if (!std::cin) {
			std::cin.clear();
			std::cin.ignore();
		}
		else if(inputted_n>19) {
			std::cout << "n does not support value greater than 19 currently.. due to loss of precision";
			Sleep(1000);
			std::cin.get();
			std::cin.get();
		}
		else if (inputted_n < 0) {
			std::cout << "Oops, 'n' cannot be negative!";
			Sleep(1000);
			std::cin.get();
			std::cin.get();
		}
		else
			validator = false;
	}

	validator = true;
	while (validator) {
		system("cls");
		std::cout << "Range for 'n': " << inputted_n;
		std::cout << "\nRange for 'r': ";
		std::cin >> inputted_r;
		if (!std::cin) {
			std::cin.clear();
			std::cin.ignore();
		}
		else if (inputted_r > inputted_n) {
			std::cout << "'r' cannot be greated than n!";
			Sleep(1000);
			std::cin.get();
			std::cin.get();
		}
		else if (inputted_r < 0) {
			std::cout << "Oops, 'r' cannot be negative!";
			Sleep(1000);
			std::cin.get();
			std::cin.get();
		}
		else
			validator = false;
	}

	bool run_time = true;
	int iteration = 0;
	unsigned int answer, no_of_regular_questions = 0, no_of_easy_questions = 0, is_correct = 0;
	std::string taken_answer;
	int taken_answer_int;
	std::vector <unsigned int> invalid_attempts, incorrect_attempts;
	std::vector <double> duration_of_attempts;
	std::vector <bool> given_up;

	while (run_time) {

		iteration++;
		if (iteration > no_of_questions) {
			run_time = false;
			break;
		}

		invalid_attempts.push_back(0);
		incorrect_attempts.push_back(0);
		duration_of_attempts.push_back(0);
		given_up.push_back(false);

		int n = random(1, inputted_n);
		int r = random(0, inputted_r);

		while (r > n)
			r = random(0, inputted_r);

		answer = (factorial(n)) / (factorial(r)*factorial(n - r));

		start = std::clock();
		validator = true;
		while (validator) {
			system("cls");
			std::cout << iteration << ") Find nCr if n = " << n << ", r = " << r << "\n    [type \"answer\" to give up]" << "\nAnswer(";
			if (is_correct == 1)
				std::cout << "CORRECT";
			else if (is_correct == 2)
				std::cout << "INCORRECT";
			else if (is_correct == 3)
				std::cout << "INVALID";
			else if (is_correct == 4)
				std::cout << "GIVEN-UP";
			std::cout << "): ";
			std::cin >> taken_answer;

			transform(taken_answer.begin(), taken_answer.end(), taken_answer.begin(), toupper);
			if (taken_answer == "ANSWER") {
				validator = false;
				given_up[iteration - 1] = true;
				is_correct = 4;
				std::cout << "\nAnswer was:  " << answer;
				std::cin.get();
				std::cin.get();
			}

			else {

				try {
					taken_answer_int = stoi(taken_answer);
				}
				catch (...) {
					taken_answer_int = -1;
				}


				if (taken_answer_int < 1) { // if negative or if zero
					is_correct = 3;
					invalid_attempts[iteration - 1]++;
				}
				else if (taken_answer_int == answer) {
					validator = false;
					is_correct = 1;
				}
				else {
					is_correct = 2;
					incorrect_attempts[iteration - 1]++;
				}
			}
		}
		duration_of_attempts[iteration-1] = (std::clock() - start) / (double)CLOCKS_PER_SEC;
		duration_of_attempts[iteration-1] = trunc(duration_of_attempts[iteration-1]) + (round((duration_of_attempts[iteration-1] - trunc(duration_of_attempts[iteration-1])) * 10) / 10);

		if (r == 0 || r == 1 || r == n)
			no_of_easy_questions++;
		else
			no_of_regular_questions++;
	}
	
	system("cls");
	std::cout << "You've answered " << no_of_regular_questions + no_of_easy_questions << " question";
		if (no_of_questions > 1)
			std::cout << "s";
	std::cout<<". Of which " << no_of_easy_questions << " of them "; 
	if (no_of_easy_questions == 1)
		std::cout << "is";
	else
		std::cout << "are";
	std::cout<<" considered as \"easy\"";
	std::cout << "\n You had given up: ";
	if (((no_of_questions)-(no_of_easy_questions + no_of_regular_questions)) == 0)
		std::cout << "ZER0!";
	else
		std::cout << ((no_of_questions)-(no_of_easy_questions + no_of_regular_questions));
	std::cout << " times";

	std::cout << "\n\nPerformance Summary:\n\n";
	for (int i = 0; i < no_of_questions; i++) {
		std::cout << "Question (<" << i << ">):";
		if (given_up[i])
			std::cout << "  GIVEN UP..\n\n";
		else {
			std::cout << "  Failed Attempts = (";
			if (incorrect_attempts[i] == 0)
				std::cout << "ZER0!";
			else
				std::cout << incorrect_attempts[i];
			std::cout << "),  Invalid Attempts = (";
			if (invalid_attempts[i] == 0)
				std::cout << "ZER0!";
			else
				std::cout << invalid_attempts[i];
			std::cout << "),  \n\tQuestion was answered in " << duration_of_attempts[i] << " seconds\n\n";
		}
	}

	std::cin.get();
	std::cin.get();
	std::cin.get();

	return 0;
}

[Thomas1965]

There's a lot of repetition there.
I would write a separate function to read an int an certain range.

int random(int n, int m) 
{
  return rand() % (m - n + 1) + n;
}

What will happen when n = 2 and m = 1 ?

[Manga]

why line 28 and then line 33?

It seems you could just have line 28 and set to true...

[Manga]

lines 44 and 50 have no affect on the program and can be removed.

[Manga]

line 55 change to break;

remove line 58.

[Manga]

You have repeated the same things again...

remove those sleep(1000)... and again you don't need to set validator to false just to make it true again. change to break; and remove the change to true.

[Manga]

ok so it seems we never really need to set validator to false. You can remove that variable completely. Your while loops should look like this...

while (true) {
// bunch of code
else //time to quit
break;
}

[Satan]

@Manga, The sleep is to make sure the user at least gets a glimpse of the error message
For some reason cin.get() is not pausing because it is reading something. Do you know an alternative?

Thanks for the suggestion I think while(true) and break statements would be better.

@Thomas but the inputs have different clauses and variables.. Can you give an example?

Also thanks a lot I kind of forgot that the user can give 0 as input for some reason.. hehe
So only input_n needs to be modified so it can't be 0 and it's fine otherwise. Thanks.

[dhayden]

Break it up into functions.

I agree with Thomas1965. If you write this function, then you can replace lines 33-56, 58-81, and 83-107 with calls to prompt()

// Get a value from cin such that low <= val < high.  If the user enter a value that's too
// low or too high then print lowMsg or highMsg respectively and prompt again. If they
// enter a non-number then clear the stream and prompt again.
// Returns the legal value
int prompt(int low, int high, const char *lowMsg, const char *highMsg)
{
// your code goes here
}

[Satan]

Oh I just realized that I can use pass by reference.

But still I don't understand how I will implement all the inputting in just one function. The response is different for all the inputs and for example in the case of input_r, there is an exclusive clause that it can't be higher than n whereas other inputs don't have such a clause.. *shrug*

So I would have to use 3 conditionals within the function..? But still that does nothing to the lines of code..

Or are you saying that I should use a separate function for every input (probably not)?

[JLBorges]

#include <iostream>
#include <string>

int get_int( int minv, int maxv, const std::string& prompt ) {

    std::cout << prompt << " [" << minv << ',' << maxv << "]: " ;

    int number ;

    if( std::cin >> number ) {

        if( number >= minv && number <= maxv ) return number ;
        else std::cout << "error: value is out of range\n" ;
    }

    else {

        std::cout << "error: non integer input\n" ;
        std::cin.clear() ;
        std::cin.ignore( 1000, '\n' ) ;
    }

    return get_int( minv, maxv, prompt ) ;
}

int main() {

    const int no_of_questions = get_int( 1, 100, "How many questions would you like to answer?" ) ;
    const int inputted_n = get_int( 5, 19, "Range for 'n':" ) ;
    const int inputted_r = get_int( 1, inputted_n, "Range for 'r':" ) ; // r <= n

    std::cout << "#questions: " << no_of_questions
              << "  n: " << inputted_n << "  r: " << inputted_r << '\n' ;

    // ...
}

Take it up from there.

[Manga]

put
cin.ignore();
before
cin.get();

[TheIdeasMan]

Hi,

With functions, they should be short and do 1 conceptual thing, this includes the main function. For small programs, it's reasonable for main to be a series of function calls that demonstrates the outline of the program.

Standard C++ has a variety of random number generators:

https://en.cppreference.com/w/cpp/header/random

You could use them instead of srand.

There is also facilities for sleeping a thread:

https://en.cppreference.com/w/cpp/thread/sleep_for
https://en.cppreference.com/w/cpp/thread/sleep_until

You could use them rather than the Windows Sleep function. This makes the program more portable.

[Satan]

Thanks a lot @TheIdeasMan that was helpful ;)

Oh and thank you too @Manga