Calculating freefalling object's height

Question

This is the code for my first CS1 project, I hope to be posting my code here to track my progress of learning the language of C++. I shouldn't be too shabby, since I know C considerably well and have reviewed C++ code here before.

Write a program that allows the user to enter a time in seconds and then outputs how far an object would drop if it is in freefall for that length of time. Assume that the object starts at rest, there is no friction or resistance from air, and there is a constant acceleration of 32 feet per second due to gravity. Use the equation:

$$ \text{distance} = \dfrac{\text{acceleration} \cdot\text{time}^2}{2} $$

rock.cpp:

/**
 * @file rock.cpp
 * @brief Determines the height that a rock falls (in feet) after a given time
 * @author syb0rg
 * @date 9/8/14
 */

#include <iostream>
#include <cmath>

int main(void)
{
    // initialize variables on declaration
    double time = 0;
    double acceleration = 32.174;

    std::cout << "Enter the time in seconds that a rock will fall: ";
    std::cin >> time;

    double feetFallen = (acceleration * pow(time, 2)) / 2;

    std::cout << "The rock fell " << feetFallen << " feet." << std::endl;
}

Answer 1

Your code is both short and good, so there's not much to say.

Header organization

It's a good idea to keep your headers organized. Keeping them alphabetically ordered allows you to spot accidental duplicates.

main() signature

You don't need to put void in the parentheses. It's very much a C-ism, and is not necessary in C++. Empty parentheses also means the function takes no arguments.

const variables and constants

acceleration is a constant, and should be marked constexpr to indicate it is a compile-time constant. Please note that constexpr is only available in C++11 or later. If you have to use an earlier version, you can replace it with static const for largely the same effect. feetFallen should be marked const, since you only read from it after it is initialized.

Marking variables const and constants constexpr limits the number of moving parts in your application, making it easier to reason about and keep correct.

Unnecessary comment

Your comment about initialization is not necessary. The fact that you initialize variables the same time they are declared is obvious from reading the code.

Invalid user input

time will contain the value 0 if your user inputs non-numeric input. If you want to constrain the user to numeric-only input, you can loop until the user enters only a valid number. Inspired by this StackOverflow answer, your solution could look like this:

while(!(std::cin >> time)){
    std::cin.clear();
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    std::cout << "Invalid input.  Please enter a number: ";
}

Answer 2

Thanks to the associative property of multiplication, and the fact that you're working with all double values here, you don't need parentheses around acceleration * pow(time, 2), so instead of:

double feetFallen = (acceleration * pow(time, 2)) / 2;

You can write:

double feetFallen = acceleration * pow(time, 2) / 2;

Of course, if some of the operands had been int values, you'd have to be careful.

Answer 3

• No void parameters are needed in C++, unlike in C.

• You can improve variable scope a bit more. If time is just to be inputted by the user, then you can declare it between the prompt and input.

  std::cout << "Enter the time in seconds that a rock will fall: ";
  double time;
  std::cin >> time;
  

  In addition, consider accepting a command line argument if one is given (other than the executable). Otherwise, you can still have the user input a time via cin.

  int main(int argc, char* argv[])
  {
      double time;
  
      // the executable is considered an arg
      if (argc == 1)
      {
          std::cout << "Enter the time in seconds that a rock will fall: ";
          std::cin >> time;
      }
      // other args are given
      else
      {
          // take the second arg only
          // the string has to be converted
          time = std::atof(argv[1]);
      }
  
      // ...
  }
  

• Assuming acceleration is not meant to change, you can make it const:

  const double acceleration = 32.174;
  

• You probably can avoid a slight performance hit with pow() here by just doing time * time:

  double feetFallen = (acceleration * time * time) / 2;
  

Answer 4

If you really want the code to execute efficiently, you should write

double feetFallen = 0.5 * acceleration * pow(time, 2);

instead of

double feetFallen = (acceleration * pow(time, 2)) / 2;

This lets the compiler embed the constant (16.087) instead of two constants (0.5 and 32.174) in the binary, and saves a multiplication at runtime. This is true whether or not you declare acceleration as const or constexpr — although it's good practice to mark it as a constant anyway. Personally, I would also rename acceleration in a way that indicates its units.

You might think that writing time * time would be more efficient than pow(time, 2), but it turns out that both Clang and GCC optimize pow(time, 2) as a simple multiplication instead of a function call when -O1 is enabled. So, it's really just a matter of preference.

Answer 5

A small point added to what has already been said: don't use std::endl, use '\n' unless you are really sure that you want to force std::cout to flush; and in that case also think about why you want it to flush.

std::cout << "The rock fell " << feetFallen << " feet.\n";