patterncppMinor
Calculate elapsed time from input times without using conditional statements
Viewed 0 times
elapsedwithoutstatementstimeconditionalinputcalculateusingtimesfrom
Problem
I have just started programming and we are being taught C++. After our 2nd lecture we were given an assignment to make a program that would take two values of time from the user (0 `#include
#include
#include
void main(void)
{
clrscr();
int ih,im,fh,fm,e,a,b;
cout>ih;
cout>im;
cout>fh;
cout>fm;
a=ih*60+im;
b=fh*60+fm;
e=abs(((780+a)/(781+b))*720-abs(a-b));
cout
I see several problems with my code. For one my code doesn't look nice and I would like to include hours and time in hr:min format. Furthermore, I think there are exceptions for which my code won't work. How can I sort these problems?
#include
#include
void main(void)
{
clrscr();
int ih,im,fh,fm,e,a,b;
cout>ih;
cout>im;
cout>fh;
cout>fm;
a=ih*60+im;
b=fh*60+fm;
e=abs(((780+a)/(781+b))*720-abs(a-b));
cout
I see several problems with my code. For one my code doesn't look nice and I would like to include hours and time in hr:min format. Furthermore, I think there are exceptions for which my code won't work. How can I sort these problems?
Solution
You may want to read Why is using namespace std bad practice?
Use whitespace to separate logical blocks of code:
Now you have an includes block and a code block. This makes it easier to see where the function starts.
I also changed
Try to avoid undescriptive names for variables. Abbreviations make it easier to write code, but they slow down reading code. You'll find that you spend more of your time reading code (including your own) than you do writing it.
I prefer to separate words in variable names with underscores. Another common notation is camelCase, which capitalizes words after the first.
I don't see the
If you're willing to use a space rather than a colon as the delimiter, you can read both on the same line in a relatively straightforward fashion. There are methods that can handle a colon but they tend to require conditionals or more advanced data structures. E.g.
I also added additional whitespace around all the tokens. The compiler won't care, but it makes it easier for human readers to recognize the separate parts of the code.
I'd write this
or actually
But maybe you haven't covered the shorthand assignment operators yet.
I also declared a constant (which you may not have met yet) to hold the "magic value" 60. See how it makes it more obvious what 60 represents? If you don't want to use a constant yet, just leave off the
The constant would also make it easier to change things if we switched to hundred minute hours. That seems unlikely in this case, but there are many other situations where that is valuable. Note that we'd only have to change 60 to 100 in one place with the constant. Without it, we'd have to switch in two places. It's even more valuable when there are many uses scattered throughout the code.
Note that this way, you don't need the temporary variables
Does this work?
It's not clear why you are doing the
Did you perhaps mean to say something like
That would allow you to subtract 9 AM from 3 PM without having to specify AM/PM. Note that it won't support an elapsed time of twelve hours or more. It does this with only your existing operators.
Also notice that I declared
See how I used a comment to explain why I was doing this weird thing? Now I don't have to figure it out again every time I read the code.
More Advanced
You probably haven't gotten here yet, since many classes don't bother to maintain good form, but most of your code should really be in a separate function. Code in
#include
void main(void)Use whitespace to separate logical blocks of code:
#include
int main(void)Now you have an includes block and a code block. This makes it easier to see where the function starts.
I also changed
void main to int main. While many compilers will accept void main, it is technically wrong. The C++ standards specify that main returns an int. Note that compilers will automatically return 0; for you. int ih,im,fh,fm,e,a,b;Try to avoid undescriptive names for variables. Abbreviations make it easier to write code, but they slow down reading code. You'll find that you spend more of your time reading code (including your own) than you do writing it.
int initial_hour, initial_minute, final_hour, final_minute;I prefer to separate words in variable names with underscores. Another common notation is camelCase, which capitalizes words after the first.
I don't see the
a and b intermediate variables as necessary. I moved my version of e lower. More on that later. std::cout > initial_hour >> initial_minute;If you're willing to use a space rather than a colon as the delimiter, you can read both on the same line in a relatively straightforward fashion. There are methods that can handle a colon but they tend to require conditionals or more advanced data structures. E.g.
strptime returns a struct as a result. I also added additional whitespace around all the tokens. The compiler won't care, but it makes it easier for human readers to recognize the separate parts of the code.
a=ih*60+im;
b=fh*60+fm;I'd write this
const int MINUTES_IN_AN_HOUR = 60;
initial_minute = initial_hour * MINUTES_IN_AN_HOUR + initial_minute;
final_minute = final_hour * MINUTES_IN_AN_HOUR + final_minute;or actually
const int MINUTES_IN_AN_HOUR = 60;
initial_minute += initial_hour * MINUTES_IN_AN_HOUR;
final_minute += final_hour * MINUTES_IN_AN_HOUR;But maybe you haven't covered the shorthand assignment operators yet.
I also declared a constant (which you may not have met yet) to hold the "magic value" 60. See how it makes it more obvious what 60 represents? If you don't want to use a constant yet, just leave off the
const keyword. It will work fine. It's just that the compiler won't keep someone from modifying the value after you set it. The constant would also make it easier to change things if we switched to hundred minute hours. That seems unlikely in this case, but there are many other situations where that is valuable. Note that we'd only have to change 60 to 100 in one place with the constant. Without it, we'd have to switch in two places. It's even more valuable when there are many uses scattered throughout the code.
Note that this way, you don't need the temporary variables
a and b. Does this work?
e=abs(((780+a)/(781+b))*720-abs(a-b));It's not clear why you are doing the
((780+a)/(781+b))*720 part. What's 780? 781? 720? Why pick those numbers? What are you trying to accomplish? This is where comments come in helpful, as they can explain what you are doing to the reader. Will you remember why you picked 780 and 781 in six months? Did you perhaps mean to say something like
// add twelve hours to the elapsed time
// then take the time modulus twelve hours to mask out the extra twelve hours
// so 3 - 9 becomes 15 - 9 = 6 which stays 6
// while 9 - 3 becomes 21 - 3 = 18 which becomes 18 % 12 = 6
const int MINUTES_IN_TWELVE_HOURS = 720;
int minutes_elapsed = (MINUTES_IN_TWELVE_HOURS + final_minute - initial_minute) % MINUTES_IN_TWELVE_HOURS;That would allow you to subtract 9 AM from 3 PM without having to specify AM/PM. Note that it won't support an elapsed time of twelve hours or more. It does this with only your existing operators.
Also notice that I declared
minutes_elapsed (my replacement for your e variable) at the location it was used. Now I don't have to go back to find out what type it is. I can see it right there. See how I used a comment to explain why I was doing this weird thing? Now I don't have to figure it out again every time I read the code.
More Advanced
You probably haven't gotten here yet, since many classes don't bother to maintain good form, but most of your code should really be in a separate function. Code in
main is not reusable without copy and paste, but the concept of reading two times and subtracting them is. So it would be better to put the code in another function or possibly two (one for input and one for calculations).Code Snippets
#include<math.h>
void main(void)#include<math.h>
int main(void)int ih,im,fh,fm,e,a,b;int initial_hour, initial_minute, final_hour, final_minute;std::cout << "Enter initial hour and minute separated by a space:";
std::cin >> initial_hour >> initial_minute;Context
StackExchange Code Review Q#77243, answer score: 6
Revisions (0)
No revisions yet.