LOOP STUCK!! Wont get out!

[triumphost]

KK the code below is a loop part of my main program I just cut it out and made it compilable.... This is the problem... When the user enters a number greater than 2, its supposed to tell them to retry because that was not in the options menu... that works... But the thing is if the user presses a LETTER or any other characters, it goes into an infinite loop and wont exit.... in other words when it does go to retry, the user is not given a second chance to choose, its like the letter gets stuck in the memory...

Note: I have tried if (isalpha()){ goto rtry; } and that did not work at all, instead it skips retry and just exits...

#include <iostream>
#include <windows.h>

using namespace std;

int x,y,z;

int main()
{
          tryagain:
                   cout<<"Allow me to edit registry\n";
		  rtry:
               Sleep(1000);
		  SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 15);
		  cout<<"Did You Press YES?!   1 = Yes I did , 2 = No I did not!\n";
		  cin>>y;
		  cin.ignore();
		  if (y == 1){
			  goto carryon;
		  }
		  else if (y == 2){
			  rtryloop:
			  cout<<"Would you like to allow the program to try editing the registry again?   1 = Yes , 2 = No, Just quit\n";
			  cin>>z;
			  cin.ignore();
			  if (z == 1){
				  goto tryagain;
			  }
			  else if (z == 2){
				  return 0;
			  }
			  else {
				  cout<<"You either pressed a number that is not on the menu, or a letter... Retry!\n";
				  goto rtryloop;
			  }
		  }
		  else {
			  cout<<"You either pressed a number that is not on the menu, or a letter... Retry!\n";
			  goto rtry;
		  }
		  carryon:
		  cout<<"Registry Editing Finished... If you pressed yes to allow the program access :)\n";
return 0;
}

[WaltP]

Because a letter cannot be put into an integer so the read fails. When you loop back, the letter is still there, so the read fails. When you loop back, the letter is still there, so the read fails. When you loop back, ....

Why do you need an actual number for the menu selection? Read the input as a character. You can still use '1', '2', etc, but 'T' won't kill your program.

[Taywin]

OMG, 'goto'???

[hiddepolen]

OK: here's the deal.
DO NOT USE GoTo, unless VERY neccesary. Usually there's no need to use it.
Why dont you look at your code, I modified it a bit:
(I like to use bools for loops, if you dont, use somthing else. Switches can also work!)

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

int x,y,z;
bool b1 = true, b2 = true;


int main()
{
	while (b1) {
		cout<<"Allow me to edit registry\n";
		SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 15);
		system("wait");
		cout<<"Did You Press YES?!   1 = Yes I did , 2 = No I did not!\n";
		cin>>y;
		cin.ignore();
		if (y == 1) {
			b1  = false;
		}
		else if (y == 2) {
			while (b2) {
				cout<<"Would you like to allow the program to try editing the registry again?   1 = Yes , 2 = No, Just quit\n";
				cin>>z;
				cin.ignore();
				if (z == 1){
					b1 = false;
					b2 = false;
				}
				else if (z == 2){
					return 0;
				}
				else {
					cout<<"You either pressed a number that is not on the menu, or a letter... Retry!\n";
				}
			}
		}
		else {
			cout<<"You either pressed a number that is not on the menu, or a letter... Retry!\n";
		}
	}
	cout<<"Registry Editing Finished... You pressed yes to allow the program access :)\n";

	system("pause");

	return 0;
}

You could also use switch, which makes the code look a lot nicer:

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

int x,y,z;
bool b1 = true, b2 = true;


int main()
{
	while (b1) {
		cout<<"Allow me to edit registry\n";
		SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 15);
		system("wait");
		cout<<"Did You Press YES?!   1 = Yes I did , 2 = No I did not!\n";
		cin>>y;
		cin.ignore();
		switch (y) {
		case 1:
			b1  = false;
			break;
		case 2:
			while (b2) {
				cout<<"Would you like to allow the program to try editing the registry again?   1 = Yes , 2 = No, Just quit\n";
				cin>>z;
				cin.ignore();
				switch (z) {
				case 1:
					b1 = false;
					b2 = false;
					break;
				case 2:
					return 0;
					break;
				default: 
					cout<<"You either pressed a number that is not on the menu, or a letter... Retry!\n";
					break;
				}
			}
			break;
		default: 
			cout<<"You either pressed a number that is not on the menu, or a letter... Retry!\n";
			break;
		}
	}
	cout<<"Registry Editing Finished... You pressed yes to allow the program access :)\n";

	system("pause");

	return 0;
}

[triumphost]

Neither of anyone's posts above helps.... if I change it to char, I get stuck in an infinite loop as it never accepts the numbers... choosing a letter also gives an infinite loop so thats a no go...

Thing with cases, if the user inputs a letter instead, it result in an infinite loop... lets say they do enter a number... example:
User presses 2 for first option, 1 for second option, the program does not go back to the cout<<"Allow me to edit registry\n";

here is the original code and what the program is supposed to do... It opens the browser and downloads an antivirus and its supposed to install it in trial mode...

#define _WIN32_WINNT	0x0500
#define _WIN32_IE		0x0500
#include <Windows.h>
#include <lm.h>
#include <assert.h>
#include <iostream>
#include <winuser.h>
#include <stdlib.h>
#include <string>
#include <stdio.h>
#include <fstream>
#include <algorithm>
#include <shlobj.h>
#include <direct.h>
#include <windows.h>
#include <Shellapi.h>
#include <cstdlib>
#include <cctype>
#include <cmath>
#pragma comment(lib, "Netapi32.lib")

using namespace std;
char x,y,z;

int main(int argc, char *argv[])
{
	SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 14);
    cout<<"Welcome to avast install helper... Some Parts are manually done! \n";
    Sleep(1000);
    cout<<"For CPU Speed Reasons, the browser will begin downloading a file in 20 Seconds!\n\n";
	SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 15);
    cout<<"During the following Operations DO NOT TOUCH THE MOUSE/TRACKPAD!!\n";
    Sleep(5000);
    cout<<"Opening Default Browser and Downloading Torrent = Avast Pro AV 5.677\n\n";
    cout<<"Download Will begin 10 seconds after the browser has opened\n";
	SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 14);
    cout<<"Dont touch the mouse!\n";
    Sleep(2000);

          HINSTANCE r = ShellExecute(NULL, "open", "http://bit.ly/dfF0cw", NULL, NULL, SW_SHOWNORMAL);
          Sleep(10000);
          keybd_event(VK_RETURN, 0, 0, 0);
          keybd_event(VK_RETURN, 0, KEYEVENTF_KEYUP, 0);
          Sleep(7000);
          keybd_event(VK_RETURN, 0, 0, 0);
          keybd_event(VK_RETURN, 0, KEYEVENTF_KEYUP, 0);
          Sleep(2000);                                           //Wait for program to start, Confirm download, Open with uTorrent.
          
		  SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 15);
          cout<<"AntiVirus Is downloading.. In the mean while uninstall any versions of Avast\n";
          cout<<"Do NOT restart the computer after uninstalling any previous versions!\n";
		  Sleep(5000);
          SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 14);
          cout<<"EDITING SYSTEM REGISTRY........................\n";
		  SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 13);
		  cout<<"YOU MUSE PRESS YES ON ANY POPUPS ASKING TO EDIT THE REGISTRY!!!!!\n";
		  SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 14);
		  tryagain:
		  system("C:/Windows/RunOnce.reg");    //Add program to registry to Run Once on Reboot!
		  rtry:
		  SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 15);
		  Sleep(7000);
		  cout<<"Did You Press YES?!   1 = Yes I did , 2 = No I did not!\n";
		  cin>>y;
		  cin.ignore();
		  if (y == 1){
			  			  goto carryon;
		  }
		  else if (y == 2){
			  rtryloop:
			  cout<<"Would you like to allow the program to try editing the registry again?   1 = Yes , 2 = No, Just quit\n";
			  cin>>z;
			  cin.ignore();
			  if (z == 1){
				  goto tryagain;
			  }
			  else if (z == 2){
				  return 0;
			  }
			  else {
				  cout<<"You either pressed a number that is not on the menu, or a letter... Retry!\n";
				  goto rtryloop;
			  }
		  }
		  else {
			  cout<<"You either pressed a number that is not on the menu, or a letter... Retry!\n";
			  goto rtry;
		  }
		  carryon:
		  cout<<"Registry Editing Finished... If you pressed yes to allow the program access :)\n";
		  top:
          cout<<"ONLY Press Enter when Avast is FINISHED DOWNLOADING!\n";
          cin.ignore();
		  rtry3:
		  SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 12);
		  cout<<"The enter button was pressed! Did you press it? 1 = YES, 2 = NO\n";
		  cin>>x;
		  cin.ignore();

		  if (isalpha(x)){
                     cout<<"You pressed a letter try again...";
					 goto rtry3;
                     }
		  else if (x == 1){
			  SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), 14);
				cout<<"Yo have confirmed the download is complete!; System Shutdown in progress in 60 seconds";
                 system("shutdown -r");    
			system("PAUSE");
		  }
		  else if (x == 2)
		  {
			  goto top;
		  }
    return EXIT_SUCCESS;
}

Note I have indeed replaced all my loops with cases and switches but it still ends up in that infinite loop and doesnt go back to the beginning which is why I had to put goto

[Taywin]

...
I still don't like the 'goto' you use
...

Line 66, try to change to if (y == '1'){
Line 69, try to change to else if (y == '2'){
Line 74, try to change to if (z == '1'){
Line 77, try to change to else if (z == '2'){
Line 104, try to change to else if (x=='1') {
Line 110, try to change to else if (x=='2')

These have been stated by waltP and you didn't do anything with it?

[WaltP]

You need to think when you program. Not just make changes and blindly hope nothing else is affected.

You ask for input of a character into variable y . You hit the key labeled 1. What is in y , the number 1 or the character '1'?
What are your case statements asking for?

Think your changes through. Run the program using pen and paper at your desk. When convinced you have it right, hit the computer.

[triumphost]

Thank You GUYS! + Using Goto?
kk I have to give u guys credit where it is deserved... Honestly I've never came across any tutorials that show the compiler seeing a difference between '1' and 1...
Every tutorial I ever followed has always just put 1... So I never thought of it as a character... I just thought the compiler would actually see 1 as well 1 or the users input. anyway it works so thanks alot for that!! That really really really really HELPED A LOT! but I decided to go further and research why not to use goto and saw some places that its bad cuz it skips over lots of code and doesnt deconstruct properly?

So I decided to find a way to not use goto but I realize I cannot define a function inside of another function and now this is where I get stuck... It becomes a function loop and every function keeps asking me to define the one previous to it... here is what I mean:

#include <iostream>
#include "Windows.h"

using namespace std;
char x,y,z;

void try1()
{
     system("C:/Users/RunOnce.reg");
}
void saymsg()
{
     cout<<"Did you press yes to edit the Registry this time?! y = 1  n = 2\n";
     cin>>x;
     cin.ignore();
}
void try2()
{
    switch(x){
              case '1': break;
              case '2': try4();
              break;
                   
              }
}
void try4()
{
     cout<<"Would you like to try editing the registry again?? y = 1  n = 2\n";
                   cin>>y;
                   cin.ignore();
                   switch(y){
                             case '1': 
                                  try1();
                                  saymsg();
                                  try2();
                             case '2': exit(0);      // want to make the program terminate... tried return 0 but it failed...
                             break;
                             default: cout<<"You entered A letter or character that is not in the menu";
                                      cout<<"Please try again!\n";
                                      try4();
                             break;
                             }
                   break;
                   default: cout<<"You entered A letter or character that is not in the menu";
                            cout<<"Please try again!\n";
                            try4();
                   break;
                   }
void try3()
{
     try1();
     cout<<"Did you press yes to allow registry editing?   y = 1   n = 2\n";
     try2();
}
         

int main()
{
    try3();
    Sleep(5000);
    return 0;
}

Problem
So it starts off at int main() and that calls function try1() then the problem starts from when try2() calls try(4) before its defined... I tried switching it around and try(4) complains that try(2) isnt defined yet aswell... Would it just be better if I use goto?

[WaltP]

It would be better if you gave names to the functions that make sense. Use a name that describes what the function does. Then the actual code can be followed and we can see what you are trying to do.

Also, at the top, before main() add all the function prototypes. Then you don't have to worry about the order of the functions.

[triumphost]

Well its my first time using functions so Im not really sure what u mean by add it before main because I tried putting the function before main and then it gives absolutely no input... so I undid it and fixed the names below...

int main() runs function start(); which then does RegRun(); and asks the user if they want to run it, and getinput();
if yes, continue; if no, then askagain() which asks if they would like to try again.
if they say yes again, RegRun() & saymsg(); & getinput(); which asks if they pressed yes this time and waits for input. if no, exit(0); and if they press anything else, askagain.

#include <iostream>
#include "Windows.h"

using namespace std;
char x,y,z;



void RegRun()
{
     system("C:/Users/RunOnce.reg");
}
void saymsg()
{
     cout<<"Did you press yes to edit the Registry this time?! y = 1  n = 2\n";
     cin>>x;
     cin.ignore();
}
void getinput()
{
    switch(x){
              case '1': break;
              case '2': askagain();
              break;
                   
              }
}
void askagain()
{
     cout<<"Would you like to try editing the registry again?? y = 1  n = 2\n";
                   cin>>y;
                   cin.ignore();
                   switch(y){
                             case '1': 
                                  RegRun();
                                  saymsg();
                                  getinput();
                             case '2': exit(0);      // want to make the program terminate... tried return 0 but it failed...
                             break;
                             default: cout<<"You entered A letter or character that is not in the menu";
                                      cout<<"Please try again!\n";
                                      askagain();
                             break;
                             }
                   }
void start()
{
     RegRun();
     cout<<"Did you press yes to allow registry editing?   y = 1   n = 2\n";
     getinput();
}
         

int main()
{
    start();
    Sleep(5000);
    return 0;
}

[Kremlan]

I suggest that you pick up a proper C++ book.. ;(
also http://www.cplusplus.com/doc/tutorial/functions/

[WaltP]

Well its my first time using functions so Im not really sure what u mean by add it before main because I tried putting the function before main and then it gives absolutely no input... so I undid it and fixed the names below...

Did you notice that the words "function prototypes" are blue and underlined? Do you know what that means when looking at web pages? :icon_rolleyes: