Converting between miles, meters, inches, yards and feet

Question

I am trying to use a class to convert between miles, meters, inches, yards and feet. My instructor told me that

1. I have way too many functions
2. My return statements are not storing any values and
3. My print function is empty - which I think will make more sense to me once 1 & 2 are solved.

How can I do this using less functions? Why aren't the return values storing anything?

#include <string>
#include <iostream>
using namespace std;


class DistanceConverter { //Class name
    public:
        DistanceConverter() { //Default constructor
            miles_ = 0;
        }
        DistanceConverter(double Miles) { //Overload constructor
            miles_ = Miles;
        }
        //Miles mutators and accessors
        void SetMilesToMeters(double Miles) {
            miles_ = Miles * 1609.34;
        }
        double GetMilesFromMeters() {
            return miles_;
        }

        void SetMilesToInches(double Miles) {
            miles_ = Miles * 63360;
        }
        double GetMilesFromInches() {
            return miles_;
        }


        void SetMilesToFeet(double Miles) {
            miles_ = Miles * 5280;
        }
        double GetMilesFromFeet() {
            return miles_;
        }

        void SetMilesToYards(double Miles) {
            miles_ = Miles * 1760;
        }
        double GetMilesFromYards() {
            return miles_;
        }

        //Yards mutators and accessors
        void SetYardsToMiles(double Miles) {
            miles_ = Miles / 1760;
        }
        double GetYardsFromMiles() {
            return miles_;
        }

        void SetYardsToFeet(double Miles) {
            miles_ = Miles * 3;
        }
        double GetYardsFromFeet() {
            return miles_;
        }

        void SetYardsToInches(double Miles) {
            miles_ = Miles * 36;
        }
        double GetYardsFromInches() {
            return miles_;
        }

        void SetYardsToMeters(double Miles) {
            miles_ = Miles * 0.9144;
        }
        double GetYardsFromMeters() {
            return miles_;
        }

        //Feet mutators and accessors
        void SetFeetToMiles(double Miles) {
            miles_ = Miles / 5280;
        }
        double GetFeetFromMiles() {
            return miles_;
        }

        void SetFeetToYards(double Miles) {
            miles_ = Miles / 3;
        }
        double GetFeetFromYards() {
            return miles_;
        }

        void SetFeetToInches(double Miles) {
            miles_ = Miles * 12;
        }
        double GetFeetFromInches() {
            return miles_;
        }

        void SetFeetToMeters(double Miles) {
            miles_ = Miles * 0.3048;
        }
        double GetFeetFromMeters() {
            return miles_;
        }

        //Inches mutators and accessors
        void SetInchesToMiles(double Miles) {
            miles_ = Miles / 63360;
        }
        double GetInchesFromMiles() {
            return miles_;
        }

        void SetInchesToYards(double Miles) {
            miles_ = Miles / 36;
        }
        double GetInchesFromYards() {
            return miles_;
        }

        void SetInchesToMeters(double Miles) {
            miles_ = Miles * 0.0254;
        }
        double GetInchesFromMeters() {
            return miles_;
        }

        void SetInchesToFeet(double Miles) {
            miles_ = Miles / 12;
        }
        double GetInchesFromFeet() {
            return miles_;
        }

        //Meters mutators and accessors
        void SetMetersToMiles(double Miles) {
            miles_ = Miles / 1609.34;
        }
        double GetMetersFromMiles() {
            return miles_;
        }

        void SetMetersToYards(double Miles) {
            miles_ = Miles / 0.9144;
        }
        double GetMetersFromYards() {
            return miles_;
        }

        void SetMetersToFeet(double Miles) {
            miles_ = Miles / 0.3048;
        }
        double GetMetersFromFeet() {
            return miles_;
        }

        void SetMetersToInches(double Miles) {
            miles_ = Miles / 0.0254;
        }
        double GetMetersFromInches() {
            return miles_;
        }

        //Print function
        void PrintDistances() {}

    private:
        double miles_; 
};


int main() {
    //Mile objects
    DistanceConverter MileToMeter;
    DistanceConverter MileToInch;
    DistanceConverter MileToFeet;
    DistanceConverter MileToYard;
    //Yard objects
    DistanceConverter YardToMile;
    DistanceConverter YardToFeet;
    DistanceConverter YardToInch;
    DistanceConverter YardToMeter;
    //Feet Objects
    DistanceConverter FeetToMile;
    DistanceConverter FeetToYard;
    DistanceConverter FeetToInch;
    DistanceConverter FeetToMeter;
    //Inch objects
    DistanceConverter InchToMile;
    DistanceConverter InchToYard;
    DistanceConverter InchToMeter;
    DistanceConverter InchToFeet;
    //Meter Objects
    DistanceConverter MeterToMile;
    DistanceConverter MeterToYard;
    DistanceConverter MeterToFeet;
    DistanceConverter MeterToInch;

    //Miles to X conversions
    MileToMeter.SetMilesToMeters(1);
    cout<<"Miles to meters: " << MileToMeter.GetMilesFromMeters() << endl;
    MileToMeter.PrintDistances();

    MileToInch.SetMilesToInches(1);
    cout<<"Miles to inches: " << MileToInch.GetMilesFromInches() << endl;
    MileToInch.PrintDistances();

    MileToFeet.SetMilesToFeet(1);
    cout<<"Miles to feet: " << MileToFeet.GetMilesFromFeet() << endl;
    MileToFeet.PrintDistances();

    MileToYard.SetMilesToYards(1);
    cout<<"Miles to yards: " << MileToYard.GetMilesFromYards() << endl;
    MileToYard.PrintDistances();

    //Yards to X conversions
    YardToMile.SetYardsToMiles(10.5);
    cout<<"Yards to miles: " << YardToMile.GetYardsFromMiles() << endl;
    YardToMile.PrintDistances();

    YardToFeet.SetYardsToFeet(20);
    cout<<"Yards to feet: " << YardToFeet.GetYardsFromFeet() << endl;
    YardToMile.PrintDistances();

    YardToInch.SetYardsToInches(5.3);
    cout<<"Yards to inches: " << YardToInch.GetYardsFromInches() << endl;
    YardToMile.PrintDistances();

    YardToMeter.SetYardsToMeters(11);
    cout<<"Yards to meters: " << YardToMeter.GetYardsFromMeters() << endl;
    YardToMile.PrintDistances();

    //Feet to X conversions
    FeetToMile.SetFeetToMiles(16);
    cout<<"Feet to miles: " << FeetToMile.GetFeetFromMiles() << endl;
    YardToFeet.PrintDistances();

    FeetToYard.SetFeetToYards(16);
    cout<<"Feet to yards: " << FeetToYard.GetFeetFromYards() << endl;
    FeetToYard.PrintDistances();

    FeetToInch.SetFeetToInches(2);
    cout<<"Feet to inches: " << FeetToInch.GetFeetFromInches() << endl;
    FeetToInch.PrintDistances();

    FeetToMeter.SetFeetToMeters(11);
    cout<<"Feet to meters: " << FeetToMeter.GetFeetFromMeters() << endl;
    FeetToMeter.PrintDistances();

    //Inches to X conversions
    InchToMile.SetInchesToMiles(100);
    cout<<"Inches to miles: " << InchToMile.GetInchesFromMiles() << endl;
    InchToMile.PrintDistances();

    InchToYard.SetInchesToYards(15);
    cout<<"Inches to yards: " << InchToYard.GetInchesFromYards() << endl;
    InchToYard.PrintDistances();

    InchToMeter.SetInchesToMeters(55);
    cout<<"Inches to meters: " << InchToMeter.GetInchesFromMeters() << endl;
    InchToMeter.PrintDistances();

    InchToFeet.SetInchesToFeet(12);
    cout<<"Inches to feet: " << InchToFeet.GetInchesFromFeet() << endl;
    InchToFeet.PrintDistances();

    //Meter to X conversions
    MeterToMile.SetMetersToMiles(1);
    cout<<"Meters to miles: " << MeterToMile.GetMetersFromMiles() << endl;
    MeterToMile.PrintDistances();

    MeterToYard.SetMetersToYards(100);
    cout<<"Meters to yards: " << MeterToYard.GetMetersFromYards() << endl;
    MeterToYard.PrintDistances();

    MeterToFeet.SetMetersToFeet(100);
    cout<<"Meters to feet: " << MeterToFeet.GetMetersFromFeet() << endl;
    MeterToFeet.PrintDistances();

    MeterToInch.SetMetersToInches(100);
    cout<<"Meters to inches: " << MeterToInch.GetMetersFromInches() << endl;
    MeterToInch.PrintDistances();

}

Answer 1

Internal representation

Use exactly one internal representation and use it consistently. In your code, there is a variable miles_, which stores everything but miles.

private:
    double miles_;

This is just plain wrong. Something like miles_ = Miles * 36; must never appear in any kind of software. Ever. Yet it does.

void SetYardsToInches(double Miles) {
    miles_ = Miles * 36;
}

Constants

I opted for meter as internal unit, as internal units should always be metric base units. Also, metric units are not pluralized. Don't use magic numbers, use named constants:

private:
    double meter = 0; 
    static constexpr double inchInMeter = 0.0254;
    static constexpr double footInMeter = 0.3048;
    static constexpr double yardInMeter = 0.9144;
    static constexpr double mileInMeter = 1609.344;

Only two functions per Unit

Use a getter/setter for each unit, not for each combination of units. That's where the huge amount of functions comes from.

public:
    void SetFeet(double value) {
        meter = value * footInMeter;
    }
    double GetFeet() {
        return meter / footInMeter;
    }

The setter converts the value to meter and stores it, the getter does the reverse. To convert 634 inches to miles, you can now call:

DistanceConverter distanceConverter;
distanceConverter.SetInches(634);
double foo = distanceConverter.GetMiles();

Full code #1

Here's the full example code:

#include <iostream>

class DistanceConverter {
    private:
        double meter = 0; 
        static constexpr double inchInMeter = 0.0254;
        static constexpr double footInMeter = 0.3048;
        static constexpr double yardInMeter = 0.9144;
        static constexpr double mileInMeter = 1609.344;
    public:
        void SetMeter(double value) {
            meter = value;
        }
        double GetMeter() {
            return meter;
        }

        void SetInches(double value) {
            meter = value * inchInMeter;
        }
        double GetInches() {
            return meter / inchInMeter;
        }

        void SetFeet(double value) {
            meter = value * footInMeter;
        }
        double GetFeet() {
            return meter / footInMeter;
        }

        void SetYards(double value) {
            meter = value * yardInMeter;
        }
        double GetYards() {
            return meter / yardInMeter;
        }

        void SetMiles(double value) {
            meter = value * mileInMeter;
        }
        double GetMiles() {
            return meter / mileInMeter;
        }
};


int main() {
    DistanceConverter distanceConverter;

    distanceConverter.SetMiles(1);
    std::cout << "Miles to meter: " << distanceConverter.GetMeter() << '\n';
    std::cout << "Miles to inches: " << distanceConverter.GetInches() << '\n';
    std::cout << "Miles to feet: " << distanceConverter.GetFeet() << '\n';
    std::cout << "Miles to yards: " << distanceConverter.GetYards() << '\n';
    std::cout << "Miles to miles: " << distanceConverter.GetMiles() << '\n';
}

But wait, there's more

You noticed that some functions are very similar?

void SetFeet(double value) {
    meter = value * footInMeter;
}
void SetYards(double value) {
    meter = value * yardInMeter;
}

The only difference is a constant. This calls for an enum and a lookup table:

enum class Unit { m, in, ft, yd, mi };

private:
    double meter = 0;
    static double inMeter(Unit unit) {
        switch(unit) {
            case Unit::m: return 1.0;
            case Unit::in: return 0.0254;
            case Unit::ft: return 0.3048;
            case Unit::yd: return 0.9144;
            case Unit::mi: return 1609.344;
        }
    }

And a single Get/Set function that handles all units:

public:        
    void Set(Unit unit, double value) {
        meter = value * inMeter(unit);
    }
    double Get(Unit unit) {
        return meter / inMeter(unit);
    }

Full code #2

Here's the full code for that variant:

#include <iostream>

enum class Unit { m, in, ft, yd, mi };

class DistanceConverter {
    private:
        double meter = 0;
        static double inMeter(Unit unit) {
            switch(unit) {
                case Unit::m: return 1.0;
                case Unit::in: return 0.0254;
                case Unit::ft: return 0.3048;
                case Unit::yd: return 0.9144;
                case Unit::mi: return 1609.344;
            }
        }
    public:        
        void Set(Unit unit, double value) {
            meter = value * inMeter(unit);
        }
        double Get(Unit unit) {
            return meter / inMeter(unit);
        }
};


int main() {
    DistanceConverter distanceConverter;

    distanceConverter.Set(Unit::mi, 1);
    std::cout << "Miles to meter: " << distanceConverter.Get(Unit::m) << '\n';
    std::cout << "Miles to inches: " << distanceConverter.Get(Unit::in) << '\n';
    std::cout << "Miles to feet: " << distanceConverter.Get(Unit::ft) << '\n';
    std::cout << "Miles to yards: " << distanceConverter.Get(Unit::yd) << '\n';
    std::cout << "Miles to miles: " << distanceConverter.Get(Unit::mi) << '\n';
}

But why did we create a class in the first place?

To set the value with one setter and get it with the other, of course! But we just eliminated multiple setters. So can we eliminate that class? Yes, we can:

static double Convert(Unit from, Unit to, double value) {
    return inMeter(from) / inMeter(to) * value;
}

Just create a static converter function and put everything in a namespace. No need to create an instance before conversion any more.

Full code #3

Here's the non-class version:

#include <iostream>

namespace DistanceConverter {
    enum class Unit { m, in, ft, yd, mi };

    static double inMeter(Unit unit) {
        switch(unit) {
            case Unit::m: return 1.0;
            case Unit::in: return 0.0254;
            case Unit::ft: return 0.3048;
            case Unit::yd: return 0.9144;
            case Unit::mi: return 1609.344;
        }
    }

    static double Convert(Unit from, Unit to, double value) {
        return inMeter(from) / inMeter(to) * value;
    }
}

int main() {
    using namespace DistanceConverter;
    std::cout << "Feet to meter: " << Convert(Unit::ft, Unit::m, 1) << '\n';
}

Answer 2

C++ is not Java. With this I mean, don't create classes just for fun. Why did you create a class? What does it even represent? Answer: Nothing, it is a just a collection of functions, with one variable.

You'd be better off not writing a class.

Here are some comments about your code, ignore the fact that you are using a class. I will propose another solution you can analyze later on.

Don't use using namespace std;

using namespace std; is bad practice. For small programs like yours, it is fine, but it is better if you write off that bad practice as soon as possible.

Why so many DistanceConverter objects?

Why do you need to use so many instances of DistanceConverter? Why not use 1 instance?

DistanceConverter Converter;

Converter.SetMilesToMeters(1);
cout<<"Miles to meters: " << Converter.GetMilesFromMeters() << endl;
Converter.PrintDistances();

Converter.SetMilesToInches(1);
cout<<"Miles to inches: " << Converter.GetMilesFromInches() << endl;
Converter.PrintDistances();

//...

Use '\n' instead of std::endl

std::endl outputs a new line, and flushes the stream. In everyday usage, you probably don't need to flush stdout, the OS will do it for you. But that is highly platform dependent. If you don't need to flush the stream, just use '\n', you'll save some CPU cycles.

If you do need to flush stdout, better be explicit and use `std::flush.

Be consistent with spacing

Sometimes, you are not putting spaces around <<, and sometimes you are. Please pick one, and use it all along:

cout<<"Feet to miles: " << FeetToMile.GetFeetFromMiles() << endl;
   ^^^^                                                 ^^^^^
 no spaces                                            with spaces

Why do you have 16 functions that do the exact same thing?

You literally have 16 functions that return miles_. Why not make that 1 function. Or better yet, why make miles_ private in the first place if you are going to provide getter and setters? Just use a public variable, nothing bad with those. Getters and setters are just unnecessary bloat (from Java?).

---

Ok, so what do we do about that class? We can write functions instead, making them constexpr for some increase in performance.

constexpr double miles_to_meter(double miles) noexcept {
    return miles * 1609.34;
}

constexpr double miles_to_inches(double miles) noexcept {
    return miles * 63360;
}

//...

If you feel you want to do something slightly more complicated, then you could use literals. But the literals are really nice :)

struct meter {
    double value = 0;
};

struct inch {
    double value = 0;
};

struct mile {
    double value = 0;

    constexpr operator meter() noexcept {
        return{ value * 1609.34 };
    }

    constexpr operator inch() noexcept {
        return{ value * 63360 };
    }
};


constexpr meter operator""_meter(double value) noexcept {
    return{ value };
}

constexpr mile operator""_mile(double value) noexcept {
    return{ value };
}

constexpr inch operator""_inch(double value) noexcept {
    return{ value };
}

Then you can use:

mile distance_home = 2.5_mile;
inch distance_home_inches = distance_home;
meter distance_home_meters = distance_home;

Answer 3

I like what @Rakete1111 said, nevertheless, I would like to present entirely different approach.

Standard library:

There is time conversion functions and time units available in the standard library. The header is <chrono>. I'll adapt it to the length conversion and units.

Units:

Treating everything as double is error prone and compiler is not able to enforce unit correctness. The best way (in my opinion) would be to prepare something like the following:

template <typename Period = std::ratio<1>>
class length
{
    double ticks;
    //stores count of Period ticks
public:
    //constructors

    template <typename AnotherPeriod>
    length(const length<AnotherPeriod>& l); //will perform cast

    auto count();
    //returns the integral part of ticks

    //friend declaration for convert
} 

using meter = length<>; //will default to Period=std::ratio<1>
using decimeter = length<std::deca>
using kilometer = length<std::kilo>
//etc

As you can see, length serves as a template for every other length unit. The Period represents amount of meters in one tick.

unsigned long long count();

The function above will give integral part of the internal_state. For kilometer it will return count of kilometers, for meter it will return number of meters.

Conversion between units:

Since we've dealt with the units, now lets consider the conversion.

template <typename TargetUnits, typename OriginalUnits> //omitted constraints, etc
TargetUnits length_cast(const OriginalUnits& l);

The template above is probably very confusing. Lets rewrite it like this:

template <typename TargetPeriod, typename OriginalPeriod>
length<TargetPeriod> length_cast(const length<OriginalPeriod>& l);

If it still confuses, there is much more verbose version:

template <std::uintmax_t TargetNum, std::uintmax_t TargetDenum, std::uintmax_t OrigNum, std::uintmax_t OrigDenum>
length<std::ratio<TargetNum, TargetDenum>> length_cast(const length<std::ratio<OrigNum, OrigDenum>>& l);

So, we've arrived to the stage where we can deal with it. Lets recall that std::kilo is std::ratio<1000, 1> and std::deca is std::ratio<10, 1>.

When we see this:

auto meters = length_cast<meter>(kilometer{3});

We know that user wants to convert 3 kilometers to meters. The key thing here is that no meters should be gained or lost, i.e. only units should be converted.

So we have pretty simple equation: \$OriginalPeriod*OriginalTicks=TargetPeriod*TargetTicks\$ =>

\$TargetTicks=\frac{OriginalPeriod*OriginalTicks}{TargetPeriod}\$

I'll leave the implementation as an exercise.

Sum up:

It is very hard to write code for imaginary purpose. Try to write some test case that will reflect the way you want to use it. That way you will have your sight fixed and focused. Also, search for inspiration!

Additional things to implement:

• String literals (""_m, ""_km, etc). As Jerry noted, user defined literals have to start with underscore.

• Arithmetic operators

• Stream operators

• Stricter specification for some parts

Answer 4

Yep: The other are have done a good job reviewing your code.

But I am going to flip the bus on you and suggest a completely different style.

First thing is you need to decide what units your internal storage mechanism is. Since we are scientists I would suggest SI units (but go with what you feel works for you and the smallest chunk of that unit you want to represent (micro). Then you can store the number as an exact integer with no changes when you serialize it (in human form).

So I will use micro meters

struct UnitOfLength
{
    // Distance in micro meters
    long   distance;

    // Don't want auto conversion from integer values.
    explicit UnitOfLength(long value): distance(value) {}
    explicit UnitOfLength(ing  value): distance(value) {}
};

So the next thing. Is constructing the object of this class. Integers have no type. So I would introduce types that represent the different units of measurements. These types know how to convert to a Unit of Length:

UnitOfLength Meters(int meters) {return UnitOfLength{meters * 1'000'000;};}
UnitOfLength Feet(int feet)     {return UnitOfLength{feet * 1'000'00 * 0.3048};}

So you can write these functions that look like conversion operators that all give you a unit of length back.

The other things is extracting the value in a particular unit of measurement:

long toMeeter(UnitOfLength const& meters) {return meters.distance / 1'000'00;}
long toFeet(UnitOfLength const& meters)   {return meters.distance / (1'000'00 * 0.3048);}
...

To wrap everything up in a namespace but keep very simple.

Reason

The reason to do it this simpler way is the OpenClosed principle.

Your code is Closed/Closed. Nobody else can add any conversions without modifying your class. Changes to the class mean that any other libraries that depend on your library will need to be re-compiled.

With this design we can add new conversations to the namespace re-build the library and re-distribute. There will be no breaking changes to existing code (just new functionality).