HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMajor

Converting between miles, meters, inches, yards and feet

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
yardsmilesfeetbetweenandconvertingmetersinches

Problem

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

  • I have way too many functions



  • My return statements are not storing any values and



  • 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
#include
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;

Solution

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 

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

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::c

Code Snippets

private:
    double miles_;
void SetYardsToInches(double Miles) {
    miles_ = Miles * 36;
}
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 SetFeet(double value) {
        meter = value * footInMeter;
    }
    double GetFeet() {
        return meter / footInMeter;
    }
DistanceConverter distanceConverter;
distanceConverter.SetInches(634);
double foo = distanceConverter.GetMiles();

Context

StackExchange Code Review Q#157975, answer score: 28

Revisions (0)

No revisions yet.