4
\$\begingroup\$

This is a simple program to convert miles, yards, feet, and inches appropriately. Everything intended for works but I feel my program is very redundant. How can I make it more efficient and less lines of code? I tried % but failed to find way to increment next values.

#include <iostream>
using namespace std;

//NO GLOBAL VARIABLES

int main()
{
    int m, y, f, i;
    int a, b, c;
    cout << "Enter: ";
    cin >> m >> y >> f >> i;
    cout << "You enter: " << m << "miles " << y << "yards "<< f << "feet " << i    << "inches " << endl;
    cout << "Convert the distance" << endl;

    // 2miles 1761yards 4feet 27inches
    // = 3miles 2yards 3feet 3inches

    //ensure mile is zero or positive
    if(m<0)
    {
        return 0;
    }

    //yard to mile conversion
    for(a=0; a<100; a++)
    {
        if(y<0)
        {
             return 0;
        }
        if(y>1760)
        {
            m++;
            y -= 1760;
        }
        if(y==1760)
        {
            y = 0;
            m++;
        }
        if(y<1760)
        {
            y = y;
        }
    }

    //feet to yard conversion
    for(b=0; b<100; b++)
    {
        if(f>3)
        {
            y++;
            f -= 3;
        }
        if(f==3)
        {
            y++;
            f= 0;
        }
        if(f<3)
        {
            f = f;
        }
        if(f<0)
        {
            return 0;
        }
    }

    //inches to feet conversion
    for(c=0; c<100; c++)
    {
        if(i>12)
        {
            f++;
            i -=  12;
        }
        if(i==12)
        {
            f++;
            i = 0;
        }
        if(i<0)
        {
            return 0;
        }
    }

    cout << "After conversion: " << m << "miles " << y << "yards "<< f << "feet  "   << i << "inches" << endl;
    return 0;
}
\$\endgroup\$
1
\$\begingroup\$

As you guessed, you can make your code much more performant/robust by using % instead of for loops to convert the distances.

An example with just yards you would do something like:

m = 0;
y = 1761;
m += (y / 1760); // add the number of whole miles
y = (y % 1760);  // set to the number of remaining yards

At the end of that you'll see that:

m == 1
y == 1
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! This does not seem like a Code Review to me: please read What IS a Code Review? \$\endgroup\$ – SirPython Oct 3 '15 at 0:07
1
\$\begingroup\$

This is a pretty readable first try. I get what it's doing. Here are some ways you could improve it.

Units

I'd start by determining what units you want to work in internally. This doesn't have to be the same as what you print or what the user enters. Personally, I'd choose the smallest unit so you have fewer issues with fractional values. In your case, that would be inches.

I'd take the user's input and convert each input into inches, then convert from inches to each unit you want to output. This leaves you the opportunity to add other types of conversions in the future (perhaps to metric or units like lightyears, or whatever).

In order to do that, you'll want to have constants for each type of conversion, so something like this:

const int kInchesPerFoot = 12;
const int kInchesPerYard = kInchesPerFoot * 3;
const int kInchesPerMile = kInchesPerFoot * 5280;

Functions

Next, I'd break up the functionality into different functions that each do 1 specific thing. Something like this:

int getNumberOfInches()
{
    cout << "Enter your distance as 4 integers for miles, yards, feet, and inches (such as 1 5 3 14 for 1 mile, 5 yards, 3 feet, and 14 inches): ";

    int miles, yards, feet, inches;
    cin >> miles >> yards >> feet >> inches;

    // Validate that the user entered good input. If not, return a negative
    if ((miles < 0) or (yards < 0) or (feet < 0) or (inches < 0))
    {
        return kErr_InvalidInput; // <- this should be defined as -1 somewhere
    }

    cout << "You entered: " << miles << " miles " << yards << " yards "<< feet << " feet " << inches << " inches." << endl;
    cout << "Converting the distance..." << ends;

    return (miles * kInchesPerMile) + (yards * kInchesPerYard) + (feet * kInchesPerFoot) + inches;
}

Now you have more clear instructions, and you have a quick way to determine if the user entered something valid. The code is more readable, too.

Next, you'll need a function to figure out the sum of distances in all the units. Maybe something like:

void convertInchesToUnits(const int inches)
{
    int inchesRemaining = inches;
    int totalMiles = inchesRemaining / kInchesPerMile; // <- Integer divide, throws out remainder, so only whole miles
    inchesRemaining = inchesRemaining % kInchesPerMile; // <- Gets the remainder in inches

    int totalYards = inchesRemaining / kInchesPerYard;
    inchesRemaining = inchesRemaining % kInchesPerYard;

    int TotalFeet = inchesRemaining / kInchesPerFoot;
    inchesRemaining  = inchesRemaining % kInchesPerFoot;

    // Tell the user what we found
    cout << "After conversion you have: " << totalMiles << " miles " << totalYards << " yards "<< totalFeet << " feet and "   << inchesRemaining << " inches." << endl;
}

Your main() function then becomes simpler:

int main()
{
    int inches = getNumberOfInches();
    if (inches >= 0)
    {
        convertInchesToUnits(inches);
        return 0; // Indicates success
    }

    return inches; // Indicates failure
}
\$\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.