problem with a repeater

I have a problem with my program that repeat names,it work correctly first then it glitch out when you use the program loop many times,and
1
2
3
4
5
6
 else

    {
       cout << "denied";
       delay(3);
    }

keep repeating itself until I close the program.

here's the code.
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
56
57
58
59
60
61
62
63
64
65
66
67
  #include <iostream>
#include <string>
#include <time.h>
#include <conio.h>
#include <windows.h>
#include <stdlib.h>


using namespace std;

void repeation(int maxx);

void delay (float de)
{
    Sleep(de*1000);
}
void command(string choose)
{
    if(choose == "yes" || choose == "YES"){
        int number;
        cout << "enter the number of repeation ";
        cin >> number;
        cout << "press any key to stop the repeat";
        delay(2);
        repeation(number);
        ;
    }
    else

    {
       cout << "denied";
       delay(3);
    }
}


void repeation(int maxx)
{
    int numb = 0;
    while( maxx > numb && !kbhit())
        {cout << "any thing";
        delay(0.01);
        numb ++;
 }
   numb = 0;
   delay(2);
        }



int main()
{
    delay(1);
    while (1){
//............................------------------------------...........................;
    cout << "                     repeatation program                                       "<<endl;
    cout <<"                 ____________________________                                   \n\n";
    cout <<"press any yes to choose to repeat " << "= ";
    string choice;
    cin >>choice;
    cout <<"________________________________ \n\n\n";
    command(choice);
    system("cls");
    }

}


so can you check it and tell me if something wrong with my code btw I tried
to remove conio.h and it did nothing.
Last edited on
Explain exactly what you are inputtng into the program to cause it to "glitch" out.

Is it possible you're accidentally putting a letter or something invalid into your int number?

On line 22, check to make the the input is valid by changing line 22 to
1
2
3
4
5
6
7
if (!(cin >> number))
{
    cout << "Invalid number\n";
    number = 0;
    cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    cin.clear();
}


Also #include <limits> at the top.

and then try to repeat the issue.
The combination of cin.ignore() + cin.clear() resets your cin buffer into a clean state to allow it to correctly receive input again.
Last edited on
the glitch was that every thing was missed up in the console and denied keep repeating,
so I tried your solution now it says access denied for a few seconds then every thing is back


to normal you can't so program and just type character or a very big number like(2000*10^9)

when the program ask you for a number and you see the glitch and thank you.
Your code works fine for me. I had to beat on it a little to get it working (not on a windows system) but it worked once I fixed the OS specific junk.

Maybe try my version? It will quickly run and tell you if it works or not, since I wrote my own sleep that runs instantly (after getting tired of waiting on it). Once you see it is working you can put your delays back in.

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
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
#include <iostream> //fixed headers to c++ versions
#include <string>   
#include <ctime>
#include <cstdlib>

void sleep(int x)
{
  return;
}

using namespace std;

void repeation(int maxx);

/*void delay (float de) //code bloat
{
    sleep(de*1000);
}*/

void command(string choose)
{
    if(choose == "yes" || choose == "YES")
	{
        int number;
        cout << "enter the number of repeation ";
        cin >> number;
        cout << "press any key to stop the repeat";
        //delay(2);
		sleep(2000);
        repeation(number);
    }
    else
    {
       cout << "denied";
       sleep(3000);
    }
}


void repeation(int maxx)
{
    int numb = 0;
    while( maxx > numb) // && !kbhit())
	   {
		    cout << "any thing\n";
			sleep(10);
			numb ++;
	   }
   numb = 0;
   sleep(2000);
}



int main()
{    
    while (1)
	{
//............................------------------------------...........................;
    cout << "                     repeatation program                                       "<<endl;
    cout <<"                 ____________________________                                   \n\n";
    cout <<"press any yes to choose to repeat " << "= ";
    string choice;
    cin >>choice;
    cout <<"________________________________ \n\n\n";
    command(choice);
    system("cls");
    }

}


now, it may not do what you expect. it repeats anything N times if you type in yes, which is strange behavior, but ok.
Last edited on
thank you for your time but it have the same problem @jonnin,

and thanks to ganado your code make the file works and the glitch apears just a second

and disappear.
Yes, I did not change anything, I just made it run without the sleeps, and it worked on my machine if I entered integers for the integer so I stopped there. I did not realized you wanted to check for bad inputs.

you may still want to fix your C headers for good practice.
Last edited on
@ganado when I tried your solution if
1
2
3
4
5
6
7
 (!(cin >> number))
{
    cout << "Invalid number\n";
    number = 0;
    cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    cin.clear();rs 
}


it says invalid,and the glitch appears for a few seconds and disappear when I write an invalid character.
Last edited on
Hello ahmedddddddd,

Sorry this is long it kink of got away from me, so it is in two parts.

Looking at your program it is easy to see that it is written for Windows.What is not easy to figure out is what IDE you are using, if any, as it would help in answering your questions.

I am not saying that your program is totally wrong as it can be made to work, but there are some better way to do what you did.

The following code works and I will use it to explain where you are having problems.

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
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
#include <cctype> // <--- Added. For "std::tolower()" and "std::toupper()".
#include <iostream>
#include <iomanip>  // <--- Not needed in this program, but will be used in the future. For "std::setpercision()" and "std::setw()".
#include <string>
#include <conio.h>
#include <windows.h>
#undef min
#undef max
#include <limits>
//#include <ctime>
#include <cstdlib>

#include <chrono>
#include <thread>

//using namespace std;  // <--- Best not to use.
// A recent post that is worth reading. http://www.cplusplus.com/forum/beginner/258335/

//void repeation(int maxx); // <--- Not needed if you put the functions in this order.

void delay(DWORD de)
{
	Sleep(de);
}

void repeation(int maxx)
{
	int numb = 0;

	while (maxx > numb && !_kbhit())
	{
		std::cout << "\n  any thing";

		delay(1000);

		numb++;
	}

	if (_kbhit())
		_getch();

	//numb = 0; // <--- Not needed. taken care of by line 28.

	delay(2000);
}

void command(char choose)
{
	constexpr size_t MINNUM{ 0 }, MAXNUM{ 10 };

	if (choose == 'y' || choose == 'Y') // <--- Should be done in "main" not here.
	{
		int number;

		std::cout << "\n Enter the number of repeation: ";
		std::cin >> number;

		//if (!(std::cin >> number))
		//{
		//	std::cout << "\n    Invalid number!\n";
		//	number = 0; // <--- Set to zero here for the function call on line 108.
		//	std::cin.clear();
		//	std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
		//}
		//else
		//	std::cout << "\n Press any key to stop the repeat.\n";

		//while (!std::cin)
		//{
		//	std::cout << "\n    Invalid number!\n";

		//	number = 0; // <--- Set to zero here for the function call on line 108.

		//	std::cin.clear();
		//	std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');

		//	std::cout << "\n Enter the number of repeation: ";
		//	std::cin >> number;
		//}

		while (!std::cin || (number < MINNUM || number > MAXNUM))
		{
			if (!std::cin)
			{
				std::cout << "\n    Invalid number!\n";

				number = 0;

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

				std::cout << "\n Enter the number of repeation: ";
				std::cin >> number;
			}
			else
			{
				std::cout << "\n Number out of range! Range is 1 - 10.\n";

				std::cout << "\n Enter the number of repeation: ";
				std::cin >> number;
			}
		}

		std::cout << "\n Press any key to stop the repeat.\n";

		delay(2000);

		repeation(number);

	}
	else
	{
		std::cout << "\n    Denied.";
		delay(1000);
	}
}

int main()
{
	std::this_thread::sleep_for(std::chrono::milliseconds(1000));  // Requires header files "chrono" and "thread".

	while (1)
	{
		//............................------------------------------...........................;
		std::cout << "\n\n                     repeatation program                                       " << std::endl;
		std::cout << "                 ____________________________                                   \n\n";
		std::cout << "\n Enter y to choose to repeat: ";

		char choice;

		std::cin >> choice;

		std::cout << "________________________________ \n\n";

		command(choice);

		if (std::toupper(choice) != 'Y')  // <--- An alternative for the if condition. And, if moved up, an alternative to the if/else in the "command" function.
		{
			// <--- A "cout" statement of your choice. e.g., "Denied.".
			break;
		}

		system("cls");
	}

	return 0;
}


Starting with the header files.As jonnin mentioned you should fix your header files.Mixing C and C++ header files is not always a good idea.Sometimes you have no choice like with "Windows.h" many of the C header files do have a C++ version that starts with "c" followed by the C file name and no extension like "cstdlib".At http ://www.cplusplus.com/reference/ click the "+" next to "C Library" to see the C++ equivalent of the C header file. By clicking on the file name you can see what is available for the given header files.

The "cctype" is optional for this program.I included and used it as an example of what could be done.

"iomanip" is not needed for this program, but you might as well get use to seeing and using it because it will be used in the future.

"conio.h":

salem c once wrote :

#include<conio.h>
Obsolete since 1990, when the world stopped using DOS as a primary operating system.


Another problem here is that your "conio.h" may be old enough that it might contain functions that are no longer available in the newer versions of the header file.And even then the header file is for backward compatibility and not meant to be used in newer programs.

A couple of years ago when I first tried to mix "Windows.h" and "limits" trying to use the linestd::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); I had a problem with the "max()" that "limits" wanted to use.As I found out later "Windows.h" defines the macros of "min" and "max" as:
#define min(a, b) (((a) < (b)) ? (a) : (b)) and
#define max(a, b) (((a) > (b)) ? (a) : (b))
Unfortunately the "min" and "max" in "limits" is different than the above code hence the#undef s before you include "limits". This will let the "std::cin.ignore(...)" work properly.

"ctime" or "time.h", you should use the "ctime", is not used in your program.As such it is not needed and is just putting extra code into your program that you are not using.

The header fileschrono and thread are there to show you an example replacement for the "Sleep()" you are using.

For usingusing namespace std; read the link.It is worth the time as is http ://www.lonecpluspluscoder.com/2012/09/22/i-dont-want-to-see-another-using-namespace-xxx-in-a-header-file-ever-again/

Line 19 the prototype can be deleted if you order your functions as I have done.The reason you needed the prototype is because you were trying to call a function that had not yet been defined.This way "delay" is define and compiled before "repeation" used it and both functions are defined and compiled before "command" uses them.If you are going to prototype a function you might as well prototype all the functions just to be safe.Also prototyping all functions means that you do not have to worry about the order the functions are done in.

In the "delay" function Microsoft has this to say about "Sleep" :

void Sleep(
DWORD dwMilliseconds
);


Where "DWORD" istypedef unsigned long DWORD; on my computer/IDE/header files. It is possible that this could be different with a different compiler and header files.Your use of sending a "float", a "double" is the preferred floating point type these days, to the function which eventually is stored in a "DWORD" could cause possible data loss when the decimal portion is dropped to store the whole number.The one place you used the "float" by sending "0.01" to the function ended up with a value of(10) and (10) milliseconds is not enough time to see any delay.

By reworking the "delay" function you could just as easily replace "delay" in the program with "Sleep" and eliminate the need for the function.

End part 1.
Part 2.

In the "repeation" function there is not much I had to change there.Thekbhit() and getch(); I had to change to_kbhit() and _getch(); for my compiler to accept them with out any problem.With out the underscore I get the message : "
Error C4996 'kbhit' : The POSIX name for this item is deprecated.Instead, use the ISO C and C++ conformant name : _kbhit.See online help for details. problem with a repeater f : \vs 2017\problem with a repeater\problem with a repeater\main v1.cpp 30". The part in bold is the most important part. The rest will be different on your computer.

I also had to add line 39 and 40 to eat what was entered for the "_kbhit()" so that it would not show up later in the program as it did.

Line 42 is unnecessary because you never use "numb" before the functions ends at which time you loose the variable "numb" when the function ends.The next time the function is call you define "numb" and set it to zero before you use it and everything works fine.

In the function "command" the first line is a better way to deal with the uncommented while condition than using magic numbers, as they are called.

As the comment says the if/else should be in "main" and not here.By the time this function is called you should already know that you need to be here.

The first if statement is based on Ganado's suggestion although I added the else part. If you can not figure out why let me know. It took me a little thought before I finally understood how it works.

The first while loop just checks for something other than a number being entered into "number". As formatted input "cin" is expecting a numeric value to be entered and when something that is not a number is entered it causes the stream to fail which needs to be dealt with.

The second while loop is much like the first while loop except the it also checks that the number entered is with in the range of "MINNUM" and "MAXNUM".This program is not a good example for using the constant variables, but it can give you an idea for the future.

The only issue I found with Ganado's suggestion is the order of the std::cin.ignore(...); and the std::cin.clear();. When I tried the "ignore" before the "clear" this did not seem to work, but when I reversed the order it worked fine. Not sure why, but I have always seen and used the "clear" before the "ignore".

Should you use the if/else statement you will need to put a comment on line 106 for it to work properly.

For "main" the first line is an alternative to using the "Sleep()" function which is specific to Windows and not everyone can use it as jonnin has mentioned.This line is also a more portable form of code that anyone can use.

With in the line ofstd::this_thread::sleep_for(std::chrono::milliseconds(1000)); the only two parts you need to worry about are using either "milliseconds" or "seconds" and the number in(). The number in() can either be zero or greater for "milliseconds" where 500 is 1/2 second, 1000 is one second and 1500 is one and 1/2 seconds.For seconds this is a whole number zero or greater like 1, 2, 3 etc. and there is no partial seconds.

I believe the "glitch" you are trying to describe starts with the while loop.As you have written it is an endless loop with no way out.The if statement I added fixes this problem.Should you move the if statement on line to somewhere above line 135 you will be doing your check in "main", where it should be, and not the "command" function where it is now.

The "system" anything should be avoided as it could leave your program and computer vulnerable to attack.

The last point is the variable "choice". As a "std::string" you can either accept "yes" or "YES" as the only two possible choices and consider anything else as invalid or have a very large if statement trying to account for every possible way of spelling "yes" with lower case and upper case letters and that does not consider any way of misspelling the word.It is better to define "choice" as a "char" and only accept one letter to deal with.A point to consider here is to keep it simple for everyone.

Some of the code I have is not required that you use it, but offered as a suggestion or alternative to what you have done.

Hope that helps,

Andy
ok I dont undrstand anything but thank you very very much I will learn then see you tobic so I can see if I will understand
https://www.photobox.co.uk/my/photo?album_id=5710127383&photo_id=502154338686
I genrated this problem when I typed a number and a letter in the number of repeatation
Hello ahmedddddddd,

Just to let you know your link did not work for me. I do not know if it is out dated or expired or maybe I have to sign up with the web site. It could be either or a combination of problems.

Having worked with C++ for awhile all the code posted here makes sense to me, but I realize that it may not be understandable to people new to the code and syntax.

If you do not understand something let me know what that is and I will try to explain it better.

Andy
Topic archived. No new replies allowed.