Anybody have any remarks on my code?

[TheGoldenCoder]

Does anybody have any remarks on my code? It's a countdown style timer. (Please note I will be adding comments soon)

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

int main(){
	using namespace std;
	int timeAmount, minCounter, secReset, hourCounter;
	minCounter = 0;
	hourCounter = 0;
	string ums;
	cout << "Do you want to set the timer in (hou)rs, (min)utes or (sec)onds?" << endl;
	getline(cin, ums);
	if (ums == "sec" || ums == "Sec"){
		cout << "Enter the number of seconds you want this timer to run: ";
		cin >> timeAmount;
	}
	else if (ums == "min" || ums == "Min"){
		cout << "Enter the number of minutes you want this timer to run: ";
		cin >> timeAmount;
		timeAmount = timeAmount * 60;
	}
	else {
		cout << "Enter the number of hours you want this timer to run: ";
		cin >> timeAmount;
		timeAmount = timeAmount * 3600;
	}
	system("CLS");
	while (timeAmount >= 1 || minCounter >= 1){
		minCounter = 0;
		hourCounter = 0;
		secReset = 0;
		while (timeAmount >= 3600) {
			timeAmount = timeAmount - 3600;
			hourCounter++;
			secReset = secReset + 3600;
		}
		while (timeAmount >= 60) {
			timeAmount = timeAmount - 60;
			minCounter++;
			secReset = secReset + 60;
		}
		if (hourCounter >= 1){
			cout << hourCounter << " hours, ";
		}
		if (minCounter >= 1){
			cout << minCounter << " minutes, ";
		}
		cout << timeAmount << " seconds remaining";
		timeAmount--;
		Sleep(999);
		system("CLS");
		timeAmount = timeAmount + secReset;
	}
	Beep(1000, 500);
	Beep(750, 500);
	Beep(500, 750);
	system("PAUSE");
	return 0;
}

[MiiNiPaa]

Your loop can be simplified greately:

    while (timeAmount >= 1)
    {
        system("cls");
        int seconds = timeAmount % 60;
        int minutes = timeAmount / 60;
        int hours   = minutes / 60;
        if (hours >= 1)  cout << hours << " hours, ";
        if (minutes >= 1)cout << minutes << " minutes, ";
        cout << seconds << " seconds remaining";
        --timeAmount;
        Sleep(999);
    }

And consider replacing cls and pause by direct calls to WinAPI.

Also you code does not work with alternative capitalization of input. for example it would not take "SEC" as input. Iterate over line you read and change it to all lowercase.

[YFGHNG]

Just to add on to what @MiiNiPaa said: you might wanna consider making your "ums" variable of type char to avoid those nasty lower and upper casing issues. Also try to avoid using system( anything ) because...let's put it this way. You're basically calling your manager's manager to solve a simple customer service issue (such as locating an item in the aisles).

namespaces usually go towards where the #includes are. What you did just looks weird.

For declaring variables such as yours, my preference would be something like:

unsigned int timeAmount, secReset; // can time really be negative?
unsigned int minCounter = 0;
unsigned int hourCounter = 0;

And yes, I'm aware there is a "negative time", usually seen in games when they ban you and you see that your unban date is something like 1973. IIRC that has something to do with how the time function is implemented in standard C++ and something about Linux too.

[MiiNiPaa]

namespaces usually go towards where the #includes are. What you did just looks weird.

No. using directive in global namespace should be avoided at all costs (except several very specific instances). Putting it in limited scope is lesser evil.

[YFGHNG]

Oh, I dunno. I've heard your argument and I've heard arguments that were along the lines of "std:: pollution" or similar. Is it an issue of memory allocation or what?

[TheIdeasMan]

Hi YFGHNG

using namespace std; or any other entire namespace negates the purpose of namespaces :)

That is, naming conflicts.

What I find to be irritating is that many teachers and authors (my problem is with them specifically) have this code. Beginners (every single one seems to do it) think that it is easier, and have no concept of what a namespace is. At some point they learn that it is bad and change their ways. So why can't teachers & authors teach it properly to start with ? I mean at the "Hello World" stage.

Regards :+)

[MiiNiPaa]

Is it an issue of memory allocation or what?

It is an issue of name clashing. Try to guess, why this code fails to compile:

#include <iostream>
#include <algorithm>
using namespace std;
void func1();
void func2();
int count;

int main()
{
    int i;
    for (i=0; i<10; i++) {
        count = i * 2;
        func1();
    }
}
void func1()
{
    func2();
    cout << "count: " << count;
    cout << '\n';
}
void func2()
{
    int count;
    for(count=0; count<3; count++) std::cout<< '.';
}

REad this (both accepted and next answer): http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice

In short, you will lose a load of names you will not be able to use in your program without risk of clashing with something. It is bad enough with C heritage, do not add C++ library names to the list.

[YFGHNG]

I see now. Though it's interesting that I've been used to creating custom namespaces to avoid those outlined issues without realizing why.

Because quite a while back, in an intro C++ class, my professor was just using "using namespace std;" and then a bit later he introduced custom namespaces, in which I probably wasn't paying attention to why it was a good idea to use those. But I did them anyways.