7 replies to this topic

[Jrb]

Hello everyone,
I've completed some homework for my intro to C++ class. The assignment was to create a C++ program that prompts the user for hours, minutes, and seconds and converts the timed event to seconds. I know it works, but I would like your opinion on the format and the use of variables, and to see if you would have done anything differently (i.e to make it look more professional).

#include <iostream>

using namespace std;


int main() {


	int hours, minutes, seconds;

	int hours_to_sec; 

	int mins_to_sec;

	int total_sec;


	cout << "Enter the amount of hours, minutes, and seconds in which the event happened.\n" << endl;


	cout << "Hours: "; 

	cin >> hours;


	cout << "Minutes: ";

	cin >> minutes;


	cout << "Seconds: ";

	cin >> seconds;


	hours_to_sec = hours * 3600;

	mins_to_sec = minutes * 60;

	total_sec = hours_to_sec + mins_to_sec + seconds;


	cout << "The total number of seconds for the event: " << total_sec << endl;


	return 0;

}

[WingedPanther]

I wouldn't have used hours_to_sec or mins_to_sec, instead including those values direction into the formula for total_sec. That could just be a style thing on my part. It looks good, overall.

[Jrb]

Yeah, I see what you're talking about now. Thanks :)

[Alexander]

GNU style:

#include <iostream>
using namespace std;

int main()
{
  int hours, minutes, seconds, total_sec;

  cout << "Enter the amount of hours, minutes, and seconds in which the event happened.\n" << endl;

  cout << "Hours: ";
  cin >> hours;

  cout << "Minutes: ";
  cin >> minutes;

  cout << "Seconds: ";
  cin >> seconds;

  total_sec = hours * 3600
              + minutes * 60
              + seconds;

  cout << "The total number of seconds for the event: " << total_sec << endl;

  return 0;
}

Edited by Alexander, 29 January 2011 - 10:09 PM.

[sam_l]

Usually this stuff is mostly personal preference. I'll show you how I would do it syntactically and explain.

#include <iostream>

// Removed namespace. I believe it is better to use namespace::function, instead of just function due to the posibility of multiple functions with the same name existing and you are not sure which one is being called.


int main() 

{ // I like to put my braces on a seperate line, I feel it helps to make things easier to read.

  // Since you are not defining any of these variables with an initial value I would put it all in one line as such. 

  // Also note there is no definition for anything else because it is a one time calculation that is easily done with few cycles.

  int hours, minutes, seconds;


  // See above about namespaces.

  // A note about endl, it does write and end-line character, but it also flushes the buffer which in most cases is unecessary and can cause unwanted effects.

  std::cout << "Enter the time taken. \n"; 

  

  std::cout << "Hours : ";

  std::cin >> hours;


  std::cout << "Minutes : ";

  std::cint << minutes;


  std::cout << "Seconds : ";

  std::cin >> seconds;

        

  // Alternatively you could use (((hours * 60) + minutes) * 60) + seconds

  std::cout << "Total time converted to seconds results in : "

                << (hours * 3600) + (minutes * 60) + seconds

                << "/n";    


  return (0);

}

[Jrb]

Thanks, Sam. I do like my braces on the same line as the initialization for personal reasons. For me, it helps me read it better (guess I'm weird). My professor doesn't put spaces inbetween operators (such as << or >>), and it drives me crazy. Everything was helpful though, so thanks again :)

[RhetoricalRuvim]

Quote

total_sec = hours * 60

              + minutes * 3600

              + seconds;

What?
Aren't minutes supposed to be multiplied by 60 and hours by 60^2?

[Jrb]

I just noticed that, too, Rhetorical. Lol, that's okay. I knew what he was trying to say :)