4
\$\begingroup\$

I'm trying to teach myself C++ from Stroustrup's Principles and Practice Using C++ (second edition) and I've gotten to the drill in chapter 4. The problem tells me to make a program that takes in a number with a unit of either feet, inches, meters, and centimeters, as well as log the largest and smallest numbers the user has input. I got the code to work by initializing my smallest number as 0 and making an if statement check if the input was smaller or 0 (because you can't really have 0 length).

Is there a way to either start with an infinitely larger "smallest number" variable so it always changes it in the if statement, or a way to where it has to do as few if statements as possible? Also, I'd rather not ask for an initial starting number before the while loop.

#include "std_lib_facilities.h"

using namespace std;


int main()
{
    cout << "Please input a number followed by a unit (cm, m, in, ft). Results are read in meters. Input a '|' to terminate the program." << endl;

    double x;
    string unit = " ";;
    constexpr double cm = .01;
    constexpr double in = .0254;
    constexpr double ft = .3048;
    double smallest = 0;
    double largest;
    double sum = 0;


    while(cin >> x >> unit)
    {
        if((char)x == '|')
            break;

        cout << "Value entered: " << x << unit << endl;
        if(unit=="m" || unit=="cm" || unit=="in" || unit=="ft") //converts units to meters
        {
            if(unit == "m")
            {
                x=x;                //
            }
            else if(unit == "cm")
            {
                x*=cm;
            }
            else if(unit == "in")
            {
                x*=in;
            }
            else if(unit == "ft")
            {
                x*=ft;
            }
            else
                cout << "Something went wrong." << endl;


            if(x > largest)
            {
                largest = x;
            }
            if(x < smallest || smallest == 0)
            {
                smallest = x;
            }
            cout << "The smallest number so far is: " <<smallest<< " meters\n";
            cout << "The largest number so far is: " << largest << " meters\n";

            sum+=x;
        }
        else
            cout << "Invalid unit! Try again." << endl;
    }
    cout << "The sum of all lengths is: " << sum << " meters\n";
}
\$\endgroup\$
4
\$\begingroup\$

I see some things that may help you improve your code.

Use numeric_limits

The answer to your question about a number to use for smallest is that you can use numeric_limits<double>::infinity() like this:

double smallest = numeric_limits<double>::infinity();

Every finite number is less than infinity, so this will work nicely here.

Consider using a structure

There is nothing in the code that binds the text "cm" to the constant cm, but this could be done within a structure (or a class, but I'm guessing you haven't gotten that far in the book yet).

struct Unit {
    string abbrev;
    double toMeters;
};
const Unit units[]{ 
    {"cm", 0.01},
    {"in",.0254},
    {"ft", 0.3048},
    {"m", 1} 
};

You might consider how to rewrite your code to use a structure like that instead. The advantage is that if you write the code generically, it makes it simple to modify the code to add new units of measure:

{"furlong", 201.168},

Consider breaking up the code into smaller functions

It doesn't matter a lot for a small and simple program like this one, but it's often useful to break up larger functions into a number of smaller, simpler ones. For example, you could implement a function to find the user's unit of measure in the structure shown above.

If these suggestions don't yet make sense to you, I'm sure they will by the time you finish the book. Good luck!

\$\endgroup\$
5
\$\begingroup\$

Overall, this looks pretty good. It's straightforward and readable, which is great.

Naming

One of the most important things in programming is naming things so that anyone reading the code can figure out the meaning. You've done a pretty good job, but I think you could improve it. Calling a variable x is kind of useless. You could name it something like userValue, or even input so someone reading the code at least knows from the name that it's something that's input by the user.

Your constant names are OK, but could be better. (It's great that you're using named constants at all, though!) It would be nice to add that your converting from cm or in to meters. And there are some common conventions for constants, too. Some people prefer constants to be all capital letters, and others prefer to prefix them with the letter "k" (like it's a mathematical constant). So you could name them CM_TO_METERS or kInToMeters, etc. That way if you add other types of conversions in the future (like converting to Imperial), then you might add kCmToInches or whatever.

Finding the Min and Max

You ask if there's a good constant to initialize the minimum to. I usually use the constant DBL_MAX in this case. (It's defined in <cfloat>.) If you're looking for only positive numbers, then you can use 0 as the start value for the maximum value. Then any number entered will automatically be greater than that.

Potential Problem

I see one thing that could be a problem. You declare x as a double, but the first thing you do with it is cast it to a char and check if it equals |. That seems really odd, error prone, and hard to understand. If it were me, I would probably refactor this to prompt the user to tell me what they want to do (convert another value, or quit), then after they've told me they want to enter a number, get the numeric input into x.

A Better Loop

You say you don't like having the initial prompt outside of the loop, and I agree. It's good to remind the user what input you're expecting each time through the loop. I'd probably rewrite the loop like this:

bool done = false;
do {
    cout << "What would you like to do?\n";
    cout << "1) Convert a value\n";
    cout << "2) Quit\n";
    char action;
    cin >> action;
    if (action == '2')
    {
        done = true;
    }
    else
    {
        // Your code for getting and converting a number
    }
} while (!done);

Now it will prompt the user each time through the loop, and when they decide to quit, they can simply enter "2" at the prompt.

\$\endgroup\$
3
\$\begingroup\$

Rather than attempting to initialize with a value that's outside the possible range of the inputs, I'd take a slightly different approach. Start by reading the first input from the user. Set both the largest and the smallest to that first value. Then read the next input. If it's smaller than the smallest, update the smallest (and likewise with the largest).

Alternatively, read and store all the inputs from the user. Then use std::minmax_element to find the smallest and largest items out of that collection. Note: minmax_element is new with C++11. If you need to work with older compilers, you can use std::min_element and std::max_element separately.

In theory, the latter is somewhat wasteful. In reality, with a program that's used interactively like this, there's basically no chance of a user entering enough numbers to use enough storage to care about.

I would probably also move a fair amount of the code into a "distance" class that would be able to read an input like "10 cm" or "14 ft", and store it into a distance object that overloaded operator< so you could compare distances directly. This could also overload operator<< to print the distances out nicely (e.g., could show each distance both in meters and its original format).

\$\endgroup\$

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.