zhodges10
zhodges10

Reputation: 5

How can I clean this code up by using a loop?

Basically, this program allows a user to enter a sentence and depending on the users selection, it will show the middle character of the sentence, display it uppercase or lowercase, or backwards. Simple program, but I am new to programming so that may be the problem. I would like to figure out how to use loops instead of a ton of if statements. When I try to make some loops it breaks certain parts of the code but I am sure that is because I don't properly understand them. If you have any criticism or any advice on the code, I'd be happy to hear it. Thanks in advance!

#include "stdafx.h"
#include <iostream>
#include <string>
using namespace std;


int main()
{
int sel;
string sent;
bool validinput;
int i;
int x;
int j;
int a;

cout << "Welcome to my program. Enter a sentence and select one of the options below.\n";
cout << "Enter -999 to exit the program." << endl;
cout << "============================================================================" << endl;
cout << endl;
cout << "1. Display the middle character if there is one." << endl;
cout << "2. Convert to uppercase." << endl;
cout << "3. Convert to lowercase." << endl;
cout << "4. Display backwards." << endl;
cout << "Enter a sentence: ";
    getline (cin, sent);
cout << "Selection: ";
    cin >> sel;

    if (sel < 1 && sel > 4)
    {
        cout << "Invalid input. Try again. Selection: ";
        cin >> sel;
        validinput = false;
    }
    else (sel >= 1 && sel <= 4);
    {
        validinput = true;
    }

    if (validinput == true)
    {
        if (sel == 1)
        {
            j = sent.length() / 2;
            cout << "The middle character is: " << sent.at(j) << endl;
        }

        if (sel == 2)
        {
            for (int i = 0; i < sent.length(); i++)
            {
                if (sent.at(i) >= 'a' && sent.at(i) <= 'z')
                {
                    sent.at(i) = sent.at(i) - 'a' + 'A';
                }
            }
            cout << "Uppercase: " << sent << endl;
        }

        if (sel == 3)
        {
            for (int x = 0; x < sent.length(); x++)
            {
                if (sent.at(x) >= 'A' && sent.at(x) <= 'Z')
                {
                    sent.at(x) = sent.at(x) - 'A' + 'a';
                }
            }
            cout << "Lowercase: " << sent << endl;
        }

        if (sel == 4)
        {
            for (a = sent.length() - 1; a >= 0; a--)
            {
                cout << sent.at(a);
            }
        }
    }

system("pause");
return 0;

}

Upvotes: 0

Views: 56

Answers (2)

D.H.
D.H.

Reputation: 186

i suggest using a switch. It will organize your code better. From looking at your code you seem to have used for and if wisely. But I suggest the if statements checking for the input be replaced with switch.

Upvotes: 0

HurpaDerpa
HurpaDerpa

Reputation: 140

Personally I would use the switch selection statement. I roughly did this just to explain a bit on how it can make your code more friendly and understandable.

int sel;
bool validInput = false;

    switch(sel)
    {
        case 1:
            //display middle char if there's one
        case 2:
            //convert to uppercase
        case 3:
            //convert to lowercase
        case 4:
            //display backwards
            validInput = true;
            break;
        default: //if number does not meat 1, 2, 3 or 4
            validInput = false;
            break;
    }

As you may notice, for case 1, case 2, case 3 and case 4, there's a break just to say that if the number is between 1 to 4; validInput is true.

Reference: Switch Selection Statement

Upvotes: 1

Related Questions