5
\$\begingroup\$

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 <= hr <= 12 and 0 <= min <= 60) and return the value of elapsed time.

Since conditionals statements are not introduced yet, I am trying to make the program without one.

#include<iostream.h>
#include<conio.h>
#include<math.h>
void main(void)
{
    clrscr();
    int ih,im,fh,fm,e,a,b;
    cout<<"Enter initial hour:";
    cin>>ih;
    cout<<"Enter initial min:";
    cin>>im;
    cout<<"Enter final hours:";
    cin>>fh;
    cout<<"Enter final min:";
    cin>>fm;
    a=ih*60+im;
    b=fh*60+fm;
    e=abs(((780+a)/(781+b))*720-abs(a-b));
    cout<<"the elasped time is "<<e/60<<":"<<e%60;
    getch();
}

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?

\$\endgroup\$
  • \$\begingroup\$ Did your teacher give any hints? My initial thought is go with what you know. Or, have you learned about the module operator yet? (%) \$\endgroup\$ – David Jan 11 '15 at 15:12
  • \$\begingroup\$ @David No hints ): and yes we know about modular operator \$\endgroup\$ – Hashir Omer Jan 11 '15 at 15:23
  • \$\begingroup\$ @David Are there any exceptions in my program? \$\endgroup\$ – Hashir Omer Jan 11 '15 at 15:23
  • \$\begingroup\$ I don't get how e works, and it seems like you should be subtracting, not adding. \$\endgroup\$ – David Jan 11 '15 at 15:36
  • \$\begingroup\$ Can you define exception from the title? Do you mean C++ exception with try and catch, things that can go wrong in your program or if the program is correct in all cases? \$\endgroup\$ – nwp Jan 11 '15 at 16:16
3
\$\begingroup\$

Algorithm:

This is good:

a=ih*60+im;
b=fh*60+fm;

Not sure what you are doing here:

e=abs(((780+a)/(781+b))*720-abs(a-b));

What are the 780/781/720 constants doing? If you remove all the extra stuff it should just work

e=abs(a-b); // Difference between the two times in minutes.

Then you print statement is fine:

cout<<"the elasped time is "<<e/60<<":"<<e%60;

Variable Names

I am sure you can come up with better names for your variables. The whole point is to try and make the code readable for the next person. So think up some meaningful names.

Layout

Declare one variable per line. Just before you use it. Its even better if you initialize it as you declare it. It looks neater and is easy to read.

std::cout << "Enter initial hour:";
int firstHour;
std::cin >> firstHour;
....

....
int timeDifference  = abs(firstTimeInMinutes - secondTimeInMinutes);

White Space to make it easier to read:

Add white space between operators so they stick out (and we don't have to squint to read your code)

cout<<"Enter initial hour:";
cin>>ih;

// Easier to read as:

std::cout << "Enter initial hour:";
std::cin >> ih;

Validate input.

Your code looks good but you have no error checking. If anything but a number is typed in then the whole program crashes with undefined behavior.

if (std::cin >> ih) {
    // Correctly read a value from the stream
}
else {
    std::cout << "Error: Could not read a number\n";
    exit(1);
}

Reading formatted text:

I would like to include hours and time in hr:min format.

C actually has a much better interface for reading formatted text:

int hour;
int min;
if (std::fscanf(stdin, "%d:%d", &hour, &min) == 2)
{
    // Read worked.
}

But it can be done with C++. You just need to manual check that the charter separator is a ':' character.

int hour;
int min;
char v;
if (std::cin >> hour >> v >> min && v == ':')
{
    // Read worked
}
\$\endgroup\$
  • \$\begingroup\$ What if min1=12*59 and min2 =60?? then abs(min1-min2) won't work \$\endgroup\$ – Hashir Omer Jan 12 '15 at 2:21
  • \$\begingroup\$ @HashirOmer. Why will it not work? I am obviously missing something but I am not sure what you are trying to point out. \$\endgroup\$ – Martin York Jan 12 '15 at 2:59
6
\$\begingroup\$

You may want to read Why is using namespace std bad practice?

#include<math.h>
void main(void)

Use whitespace to separate logical blocks of code:

#include<math.h>

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 << "Enter initial hour and minute separated by a space:";
std::cin >> 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).

\$\endgroup\$
  • \$\begingroup\$ The 780 part is to activate the number 720 when initial minutes >Final minutes so that I could get e=(720-(b-a)) \$\endgroup\$ – Hashir Omer Jan 12 '15 at 9:41
1
\$\begingroup\$

Okay, you seems to be well on your way, you just need to think outside the box.

void main()
{
    // Make your variables readable
    int initialHour, initialMin, finalHour, finalMin;

    // Get your input. Space things out for readability
    cout << "Enter initial hour: ";
    cin  >> initialHour;

    // Get your other inputs

    int endHour, endMin;

    endHour = abs(initialHour - finalHour);
    endMin = abs(initialMin - finalMin);

    endHour = endHour + int(endMin / 60);
    endMin = endMin % 60;

    // Again, space things out
    cout << "The elapsed time is: " << finalHour << ":" << finalMin << endl;
    getch();
}

Disclaimer: Written freehand and untested. May contain typos / syntax errors to be fixed by the asker

\$\endgroup\$
  • \$\begingroup\$ How can I use hr:min format in input? \$\endgroup\$ – Hashir Omer Jan 11 '15 at 15:52
  • \$\begingroup\$ Have you learned getline and string? \$\endgroup\$ – David Jan 11 '15 at 15:53
  • \$\begingroup\$ No, but I would like to know it \$\endgroup\$ – Hashir Omer Jan 11 '15 at 15:56
  • \$\begingroup\$ If you don't want to go outside of what you learned, and use conditionals, then using getline with string would be even worse. Nonetheless I can add it in if you'd like. It also wouldn't be good form to use it without conditionals. \$\endgroup\$ – David Jan 11 '15 at 15:57
  • 1
    \$\begingroup\$ No, by the way thanks for your valuable response \$\endgroup\$ – Hashir Omer Jan 11 '15 at 16:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.