Looking for some input!

Hey everyone, I've posted here a few times already, and I am still learning. I have just got a new code going and, although it's not super complicated, I'm still fairly proud of my code which I will list below. I had a friend of mine, mention that he would try to find another way to do the do/while loops that i have because apparently it's a little convoluted. What do you guys, think though? Any constructive feedback is highly appreciated! Thanks in advanced!

HEADER.H
1
2
3
4
5
6
7
8
9
10
#pragma once
#include "stdafx.h"
#include <iostream>
#include <string>
#include <ctime>
#include <stdlib.h>

using namespace std;

void mainMenu();


MAINMENU.CPP
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
#include "stdafx.h"
#include "Header.h"

string gender = "n";
string strOrSkill = "n";
string sizOrSpd = "n";
string hlthOrStmna = "n";
string correct = "n";

void mainMenu()
{
	do
	{
		do
		{
			cout << "1) Male or Female?" << endl;
			cin >> gender;
			if (gender == "m" || gender == "male")
			{
				gender = "Male";
			}
			else if (gender == "f" || gender == "female")
			{
				gender = "Female";
			}
			else
			{
				cout << "Enter either 'm' for male or 'f' for female..." << endl;
			}
		} while (gender != "Male" && gender != "Female");
		do
		{
			cout << "2) Strength or Skill?" << endl;
			cin >> strOrSkill;
		} while (strOrSkill != "strength" && strOrSkill != "skill");
		do
		{
			cout << "3) Size or Speed?" << endl;
			cin >> sizOrSpd;
		} while (sizOrSpd != "size" && sizOrSpd != "speed");
		do
		{
			cout << "4) Health or Stamina?" << endl;
			cin >> hlthOrStmna;
		} while (hlthOrStmna != "health" && hlthOrStmna != "stamina");
		system("CLS");
		cout << "You've chosen '" << gender << "' for your gender." << endl;
		cout << "You've chosen the '" << strOrSkill << "' stat." << endl;
		cout << "You've chosen the '" << sizOrSpd << "' stat." << endl;
		cout << "And you also chose the '" << hlthOrStmna << "' stat." << endl << endl;
		cout << "Are these stats correct?" << endl;
		cin >> correct;
		system("CLS");
	} while (correct != "y");
}



CONSOLEAPPLICATION.CPP
1
2
3
4
5
6
7
#include "stdafx.h"
#include "Header.h"

int main()
{
	mainMenu();
}


PS. I also understand a lot of programmers refrain from using SYSTEM commands, but in my learning stages, it's currently the easiest method for this trivial example.
> I had a friend of mine, mention that he would try to find another way to do the do/while loops
> that i have because apparently it's a little convoluted.

Each level of nesting adds intellectual overhead when reading code because you need to maintain a mental stack (e.g., enter conditional, enter loop, enter try, enter conditional, ...). Have you ever found a closing brace in someone's code and wondered which of the many fors, whiles, or ifs it matched? Prefer better functional decomposition to help avoid forcing readers to keep as much context in mind at a time.

Exercise common sense and reasonableness: Limit the length and depth of your functions. All of the following good advice also helps reduce length and nesting:
• Prefer cohesion: Give one function one responsibility.
• Don't repeat yourself: Prefer a named function over repeated similar code snippets.
...

Sutter and Alexandrescu in 'C++ Coding Standards: 101 Rules, Guidelines, and Best Practices'

Something like this, perhaps:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
#include <iostream>
#include <string>
#include <cctype>

std::string get_choice( std::string choice_1, std::string choice_2, char option_1 = '1', char option_2 = '2' )
{
    std::cout << "please enter either " << option_1 << " for '" << choice_1 << "' or "
              << option_2 << " for '" << choice_2 << "': " ;
    char choice ;
    std::cin >> choice ;
    choice = std::tolower(choice) ;

    if( choice == option_1 ) return choice_1 ;
    else if( choice == option_2 ) return choice_2 ;
    else return get_choice( choice_1, choice_2, option_1, option_2 ) ; // invalid input; try again
}

void menu( std::string& gender, std::string& strength_or_skill, std::string& size_or_speed, std::string& health_or_stamina )
{
    std::cout << "\n1) gender?\n" ;
    gender = get_choice( "male", "female", 'm', 'f' ) ;

    std::cout << "\n2) strength or skill?\n" ;
    strength_or_skill = get_choice( "strength", "skill" ) ;

    std::cout << "\n3) size or speed?\n" ;
    size_or_speed = get_choice( "size", "speed" ) ;

    std::cout << "\n4) health or stamina?\n" ;
    health_or_stamina = get_choice( "health", "stamina", 'h', 's' ) ;
}

bool correct( std::string gender, std::string strength_or_skill, std::string size_or_speed, std::string health_or_stamina )
{
    std::cout << "\nYou've chosen '" << gender << "', '"  << strength_or_skill << "', '"
              << size_or_speed << "' and '" << health_or_stamina << "'\nare these correct?\n" ;

    return get_choice( "yes", "no", 'y', 'n' ) == "yes" ;
}

int main()
{
    std::string gender, strength_or_skill, size_or_speed, health_or_stamina ;

    do menu( gender, strength_or_skill, size_or_speed, health_or_stamina ) ;
    while( !correct( gender, strength_or_skill, size_or_speed, health_or_stamina ) ) ;

    // ...
}



> I also understand a lot of programmers refrain from using SYSTEM commands,
> but in my learning stages, it's currently the easiest method for this trivial example.

Yes. A good programmer is, over and above everything else, a pragmatic programmer.
Last edited on
Thanks so much! I'm going through your revision of my code and learning all sorts of new tricks! I really do appreciate everything!
Topic archived. No new replies allowed.