Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

I see very few things to change, your code is easy to read.

  • You're not validating the input. What if someone enters something that's not a number?

I'd suggest you create a function that takes the prompt string and returns the entered integer. That function can do the validation, which isn't completely trivial.

Two good examples of how to do the validation:

You should be dividing with the same constant(s) that you use in the multiplication (or vice-versa). (These constants are easy to identify, I don't think giving them names is worth the effort. Just be systematic in how you use them.)

  • return(0); is unusual, it looks like a function call. return is not a function call, it's a statement. I would prefer return 0; instead.

  • Your code appears to have forgotten about minutes :-)

I see very few things to change, your code is easy to read.

  • You're not validating the input. What if someone enters something that's not a number?

I'd suggest you create a function that takes the prompt string and returns the entered integer. That function can do the validation, which isn't completely trivial.

Two good examples of how to do the validation:

  • What is the best way to do input validation in C++ with cin?

  • Good input validation loop using cin

  • (Very minor) You put a comment on the first two variable declarations, but not on the following ones. In my opinion, you chose sufficiently descriptive variable names, the comments aren't necessary. But if you do put a comment on some (or if your local coding style guidelines enforce that), do it consistently.

  • This "looks" strange:

     int days_needed = (seconds_needed / 3600) / 24;
     seconds_needed -= days_needed*86400;
    

You should be dividing with the same constant(s) that you use in the multiplication (or vice-versa). (These constants are easy to identify, I don't think giving them names is worth the effort. Just be systematic in how you use them.)

  • return(0); is unusual, it looks like a function call. return is not a function call, it's a statement. I would prefer return 0; instead.

  • Your code appears to have forgotten about minutes :-)

I see very few things to change, your code is easy to read.

  • You're not validating the input. What if someone enters something that's not a number?

I'd suggest you create a function that takes the prompt string and returns the entered integer. That function can do the validation, which isn't completely trivial.

Two good examples of how to do the validation:

  • What is the best way to do input validation in C++ with cin?

  • Good input validation loop using cin

  • (Very minor) You put a comment on the first two variable declarations, but not on the following ones. In my opinion, you chose sufficiently descriptive variable names, the comments aren't necessary. But if you do put a comment on some (or if your local coding style guidelines enforce that), do it consistently.

  • This "looks" strange:

     int days_needed = (seconds_needed / 3600) / 24;
     seconds_needed -= days_needed*86400;
    

You should be dividing with the same constant(s) that you use in the multiplication (or vice-versa). (These constants are easy to identify, I don't think giving them names is worth the effort. Just be systematic in how you use them.)

  • return(0); is unusual, it looks like a function call. return is not a function call, it's a statement. I would prefer return 0; instead.

  • Your code appears to have forgotten about minutes :-)

edited body
Source Link
Mat
  • 3k
  • 1
  • 22
  • 25

I see very few things to change, your code is easy to read.

  • You're not validating the input. What if someone enters something that's not a number?

I'd suggest you create a function that takes the prompt string and returns the entered integer. That function can do the validation, which isn't completely trivial.

Two good examples of how to do the validation:

  • What is the best way to do input validation in C++ with cin?

  • Good input validation loop using cin

  • (Very minor) You put a comment on the first two variable declarations, but not on the following ones. In my opinion, you chose sufficiently descriptive variable names, the comments aren't necessary. But if you do put a commentscomment on some (or if your local coding style guidelines enforce that), do it consistently.

  • This "looks" strange:

     int days_needed = (seconds_needed / 3600) / 24;
     seconds_needed -= days_needed*86400;
    

You should be dividing with the same constant(s) that you use in the multiplication (or vice-versa). (These constants are easy to identify, I don't think giving thethem names is worth the effort. Just be systematic in how you use them.)

  • return(0); is unusual, it looks like a function call. return is not a function call, it's a statement. I would prefer return 0; instead.

  • Your code appears to have forgotten about minutes :-)

I see very few things to change, your code is easy to read.

  • You're not validating the input. What if someone enters something that's not a number?

I'd suggest you create a function that takes the prompt string and returns the entered integer. That function can do the validation, which isn't completely trivial.

Two good examples of how to do the validation:

  • What is the best way to do input validation in C++ with cin?

  • Good input validation loop using cin

  • (Very minor) You put a comment on the first two variable declarations, but not on the following ones. In my opinion, you chose sufficiently descriptive variable names, the comments aren't necessary. But if you do put a comments on some (or if your local coding style guidelines enforce that), do it consistently.

  • This "looks" strange:

     int days_needed = (seconds_needed / 3600) / 24;
     seconds_needed -= days_needed*86400;
    

You should be dividing with the same constant(s) that you use in the multiplication (or vice-versa). (These constants are easy to identify, I don't think giving the names is worth the effort. Just be systematic in how you use them.)

  • return(0); is unusual, it looks like a function call. return is not a function call, it's a statement. I would prefer return 0; instead.

  • Your code appears to have forgotten about minutes :-)

I see very few things to change, your code is easy to read.

  • You're not validating the input. What if someone enters something that's not a number?

I'd suggest you create a function that takes the prompt string and returns the entered integer. That function can do the validation, which isn't completely trivial.

Two good examples of how to do the validation:

  • What is the best way to do input validation in C++ with cin?

  • Good input validation loop using cin

  • (Very minor) You put a comment on the first two variable declarations, but not on the following ones. In my opinion, you chose sufficiently descriptive variable names, the comments aren't necessary. But if you do put a comment on some (or if your local coding style guidelines enforce that), do it consistently.

  • This "looks" strange:

     int days_needed = (seconds_needed / 3600) / 24;
     seconds_needed -= days_needed*86400;
    

You should be dividing with the same constant(s) that you use in the multiplication (or vice-versa). (These constants are easy to identify, I don't think giving them names is worth the effort. Just be systematic in how you use them.)

  • return(0); is unusual, it looks like a function call. return is not a function call, it's a statement. I would prefer return 0; instead.

  • Your code appears to have forgotten about minutes :-)

Source Link
Mat
  • 3k
  • 1
  • 22
  • 25

I see very few things to change, your code is easy to read.

  • You're not validating the input. What if someone enters something that's not a number?

I'd suggest you create a function that takes the prompt string and returns the entered integer. That function can do the validation, which isn't completely trivial.

Two good examples of how to do the validation:

  • What is the best way to do input validation in C++ with cin?

  • Good input validation loop using cin

  • (Very minor) You put a comment on the first two variable declarations, but not on the following ones. In my opinion, you chose sufficiently descriptive variable names, the comments aren't necessary. But if you do put a comments on some (or if your local coding style guidelines enforce that), do it consistently.

  • This "looks" strange:

     int days_needed = (seconds_needed / 3600) / 24;
     seconds_needed -= days_needed*86400;
    

You should be dividing with the same constant(s) that you use in the multiplication (or vice-versa). (These constants are easy to identify, I don't think giving the names is worth the effort. Just be systematic in how you use them.)

  • return(0); is unusual, it looks like a function call. return is not a function call, it's a statement. I would prefer return 0; instead.

  • Your code appears to have forgotten about minutes :-)

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /