Speed, distance and time calculator

Question

This is a speed distance and time calculator - the comments should explain most of the odd looking code if there is any. I am looking for suggestions to improve the efficiency of the program and how better to structure it - is my approach the best from a 'best practices', efficiency and OOP point of view.

So, specifically:

• How might I improve to fit in with common best practices?
• I this the most efficient method and structure?
• Could I add or improve functions so I could more easily add features like unit conversions?

I don't want to lose any functionality. The current program calculates speed distance and time as well as showing all working which adds quite a lot of strings to store information. I am still looking to improve these parts as well.

The main reason I am asking, is to avoid bad practices and find better ways of doing what I'm doing. I am a C++ beginner but I know quite a lot.

#include <iostream>
#include <sstream>

using std::cout;
using std::cin;
using std::string;
using std::endl;
using std::ostringstream;

class Math
{ // Math Class
public:
    ostringstream s;

    // SPEED DISTANCE AND TIME CALCULATION
    double calcSpeed(double distance, double time, string& formula) // SPEED ~ Variable formula passed to the ufnction will be redefined
    {
        s << distance << " / " << time; // Adding numbers to string
        formula = s.str(); // Converting s to formula of type string
        return distance / time; // Returns result of function
    }
    double calcDistance(double speed, double time, string& formula) // DISTANCE
    {
        s << speed << " * " << time;
        formula = s.str();
        return speed * time;
    }
    double calcTime(double speed, double distance, string& formula) // TIME
    {
        s << distance << " / " << speed;
        formula = s.str();
        return distance / speed;
    }
};

int main(int argc, const char * argv[])
{
    Math math; // Creates object of Math class to be used to access members

    double firstvalue, secondvalue, result; // Numbers used for conditions and working
    int chosenCalc = 1;

    string chosenCalcStr = "speed"; // Strings used for working
    string firstParam, secondParam;
    string formula;
    string intform;

    bool quit = false;

    typedef double (Math::*FuncChosen)(double first, double second, string& third); // Typedef gives access to a function which can change if it has same parameter and return types

    FuncChosen p = &Math::calcSpeed; // p's default value

    /* Welcome Message */
    printf("If you have both other variables in the triangle you can calculate any element. Speed, distance or time.\n");
    printf("The answer given will be in the same units that you entered - if you entered m/s for speed, time will be in seconds etc.\n");

    /* While user enters something other than 4*/
    while (chosenCalc != 4 && !quit) // While user does not want to quit
    {
        printf("Enter 1 if you want to calculate speed, 2 for distance, 3 for time and 4 to exit.\n> ");
        cin >> chosenCalc;

        if (chosenCalc == 4)
        {
            quit = true;
            return 0;
        }

        /* Conditions */

        switch (chosenCalc) {
            case 1:
                p = &Math::calcSpeed; // Redefines p to explicit function at runtime according to conditions
                firstParam = "distance"; // Explanation strings
                secondParam = "time";
                formula = "Speed = distance / time";
                chosenCalcStr = "Speed"; // Type in formula calculated
                break;
            case 2:
                p = &Math::calcDistance;
                firstParam = "speed";
                secondParam = "time";
                formula = "Distance = Speed * time";
                chosenCalcStr = "Distance";
                break;
            case 3:
                p = &Math::calcTime;
                firstParam = "speed";
                secondParam = "distance";
                formula = "Time = distance / speed";
                chosenCalcStr = "Time";
                break;
        }

        /* User Input */

        cout << "\nPlease enter the value for " << firstParam << ".\n> ";
        cin >> firstvalue;
        cout << "Please enter the value for " << secondParam << ".\n> ";
        cin >> secondvalue;
        cout << "\n";

        result = (math.*p)(firstvalue, secondvalue, intform); // Double result = return value of chosen function **NON EXPLICIT (changes at runtime)** math is used as object to access public members

        if (chosenCalc != 1) { cout << "Speed = distance / time\n";} // Print formula - We don't want to print out sum twice unless we are not calculating speed and wnat general formula
        cout << formula << "\n"; // Print adjusted formula
        cout << chosenCalcStr << " = " << intform << "\n"; // Print numbers in formula
        cout << chosenCalcStr << " = " << result << "\n\n"; // Print result

        /* END */
    }

}

Answer 1

Seems like you got a good grasp of the basic concepts in C++. Good for you! I assume that you are writing C++03. Here is what I suggest that you change to make the code more clear:

• Your Math class should be a namespace. Currently, Math only has a single member variable, ostringstream s, which is shared between the various calc functions. This implies that subsequent calls to any calc functions will simply append to whatever is already in s. E.g.,

  Math math;
  
  std::string test1, test2, test3;
  math.calcTime(1.0, 0.9, test1);
  math.calcTime(2.0, 0.8, test2);
  math.calcTime(3.0, 0.7, test3);
  
  std::cout << "test1: " << test1 << std::endl;
  std::cout << "test2: " << test2 << std::endl;
  std::cout << "test3: " << test3 << std::endl;
  

  This will actually output

  test1: 0.9 / 1
  test2: 0.9 / 10.8 / 2
  test3: 0.9 / 10.8 / 20.7 / 3
  

  which is probably not what you want. So, I suggest that you convert Math into a namespace and define ostringstream s locally in each calc function.

• You're using both std::cout and printf. Stick to one of them. Since you are already invested in string streams, I suggest the former.

• The quit variable is redundant. You're already returning from main when chosenCalc == 4 is true. Speaking of which, I suggest that you get in the habit of writing 4 == chosenCalc instead. This way, you will get a nice compiler error if you accidentally write 4 = chosenCalc.

• You're using a switch statement. Excellent! Stick to it. Add a case 4 instead of having the if(chosenCalc == 4) before the switch. Even better, add a default case so that if the user presses 5, he gets a nice error message. E.g.:

  default:
      std::cout << "Invalid input. Please try again." << std::endl;
      continue;
  

• Cool that you have the hang of function pointers. They have a lot of uses. However, you can do without them in this case. Simply remove FuncChosen and call the calc functions directly inside the switch. Just move the /* User Input */ section up before the switch and you got all the arguments you need. Calling a function directly is also more efficient than calling it through a pointer (since it is one less layer of indirection).

You've already got some decent C++ code. I hope that the above will get you even further. Just comment if you need some elaboration of my suggestions.

---

Edit:: As per you comment, I've added a little extra. If you have access to a C++11 compiler you can:

• Use std::to_string instead of std::ostringstream. E.g.,

  double calcSpeed(double distance, double time, string& formula)
  {
      formula = std::to_string(distance) + " / " + std::to_string(time);
      return distance / time;
  }
  

• Use list initialization (also known as uniform initialization). E.g., int chosenCalc{1}; instead of int chosenCalc = 1;. The former does not allow narrowing conversions whereas the latter does. This can catch subtle errors.

• Prefer using over typedef. The former is more powerful.

Furthermore, you can also take advantage of the following C++14 feature:

• Use auto return type deduction. E.g.,

  auto calcSpeed(double distance, double time, string& formula)
  {
      formula = std::to_string(distance) + " / " + std::to_string(time);
      return distance / time; // Return type is deduced to be double.
  }
  

  This is a minor detail in you case. It can be really useful for more complex return types.

Lastly, I want to present a modern alternative to using output parameters. Let me emphasize that it is just an alternative. The output parameter approach is fine and which one you chose is completely up to you. The alternative is to return multiple values. Let's look at

double calcSpeed(double distance, double time, string& formula)

which returns a double and outputs to a std::string. We can instead return a std::pair consisting of a double and a std::string. I.e.,

std::pair<double, std::string> calcSpeed(double distance, double time)
{
    auto speed = distance / time; // Auto is deduced to double
    auto formula = std::to_string(distance) + " / " + std::to_string(time); // Auto is deduced to std::string.
    return make_pair(speed, formula);
}

Remember to #include <utility>. Note that I have also used C++11's auto keyword to deduce the type of speed and formula for us. Like I mentioned before, the auto keyword even works for the return type in C++14. So in C++14, we could even just write

auto calcSpeed(double distance, double time)
{
    auto speed = distance / time; // Auto is deduced to double
    auto formula = std::to_string(distance) + " / " + std::to_string(time); // Auto is deduced to std::string.
    return make_pair(speed, formula); // Return type is deduced to std::pair<double, std::string>
}

You could even let go of the temporary variables and write

auto calcSpeed(double distance, double time)
{ return make_pair(distance / time, std::to_string(distance) + " / " + std::to_string(time)); }

This is probably as concise as it gets. You can then call calcSpeed like this:

auto speed_and_formula = calcSpeed(0.5, 0.2); // Returns std::pair<double, std::string>
auto speed = speed_and_formula.first; // This is the double
auto formula = speed_and_formula.second; // This is the string

It's a little annoying to write all of this. The C++ committee thought so as well and provided the following simpler method:

double speed;
std::string formula;
std::tie(speed, formula) = calcSpeed(0.5, 0.2); // Assigns the first member of the pair to speed and the second member to formula.

This should give you a taster of C++11 and C++14.