Skip to main content
Code Review

Return to Answer

missing an include in the final code example
Source Link
nukeforum
nukeforum
  • 221
  • 1
  • 7
#include <iostream>
#include <algorithm>
#include <cmath>

using std::cout;
using std::cin;
using std::min;
using std::max;

//returns true if the variance is less than 0.01
bool valuesAreSimilar(double a, double b) {
  return std::abs(a - b) < 0.01;
}

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
    cout << "Not a value I know! Incorrect input" << "\n";
}

#include <iostream>
#include <algorithm>

using std::cout;
using std::cin;
using std::min;
using std::max;

//returns true if the variance is less than 0.01
bool valuesAreSimilar(double a, double b) {
  return std::abs(a - b) < 0.01;
}

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
    cout << "Not a value I know! Incorrect input" << "\n";
}

#include <iostream>
#include <algorithm>
#include <cmath>

using std::cout;
using std::cin;
using std::min;
using std::max;

//returns true if the variance is less than 0.01
bool valuesAreSimilar(double a, double b) {
  return std::abs(a - b) < 0.01;
}

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
    cout << "Not a value I know! Incorrect input" << "\n";
}
fix per Ruslan's explanation of cin failing before entering the while loop
Source Link
nukeforum
nukeforum
  • 221
  • 1
  • 7

Right up front, something to think about is that considering the case where the values are equal is probably unnecessary. Keep in mind you can always check if the values are equal outside of the context of these methods (and in nearly every case, this is preferable)(Thank you to Deduplicator for reminding me to address this). These are perhaps good to write for practice, but in a real code base, I recommend using std::min and std::max (credit to @MORTAL). For good recommendations in optimizing these two, check out @Greg Burghardt's answer. I'm changing the signatures to max() and min() for brevity's sake.

Update: As Ruslan pointed out, the while loop will never be entered if there is an assignment failure, so we'll move the failure message outside the loop.

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (cin.fail()) {
            cout << "Not a value I know! Incorrect input" << "\n";
            continue;
        }
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
    cout << "Not a value I know! Incorrect input" << "\n";
}

#include <iostream>
#include <algorithm>

using std::cout;
using std::cin;
using std::min;
using std::max;

//returns true if the variance is less than 0.01
bool valuesAreSimilar(double a, double b) {
  return std::abs(a - b) < 0.01;
}

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (cin.fail()) {
            cout << "Not a value I know! Incorrect input" << "\n";
            continue;
        }
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
    cout << "Not a value I know! Incorrect input" << "\n";
}

These are perhaps good to write for practice, but in a real code base, I recommend using std::min and std::max (credit to @MORTAL). For good recommendations in optimizing these two, check out @Greg Burghardt's answer. I'm changing the signatures to max() and min() for brevity's sake.

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (cin.fail()) {
            cout << "Not a value I know! Incorrect input" << "\n";
            continue;
        }
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
}

#include <iostream>
#include <algorithm>

using std::cout;
using std::cin;
using std::min;
using std::max;

//returns true if the variance is less than 0.01
bool valuesAreSimilar(double a, double b) {
  return std::abs(a - b) < 0.01;
}

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (cin.fail()) {
            cout << "Not a value I know! Incorrect input" << "\n";
            continue;
        }
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
}

Right up front, something to think about is that considering the case where the values are equal is probably unnecessary. Keep in mind you can always check if the values are equal outside of the context of these methods (and in nearly every case, this is preferable)(Thank you to Deduplicator for reminding me to address this). These are perhaps good to write for practice, but in a real code base, I recommend using std::min and std::max (credit to @MORTAL). For good recommendations in optimizing these two, check out @Greg Burghardt's answer. I'm changing the signatures to max() and min() for brevity's sake.

Update: As Ruslan pointed out, the while loop will never be entered if there is an assignment failure, so we'll move the failure message outside the loop.

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
    cout << "Not a value I know! Incorrect input" << "\n";
}

#include <iostream>
#include <algorithm>

using std::cout;
using std::cin;
using std::min;
using std::max;

//returns true if the variance is less than 0.01
bool valuesAreSimilar(double a, double b) {
  return std::abs(a - b) < 0.01;
}

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
    cout << "Not a value I know! Incorrect input" << "\n";
}
Source Link
nukeforum
nukeforum
  • 221
  • 1
  • 7

I'm just going to break this down by the method.

double similarValues(double valueBigger, double valueSmaller)
{
    if ((valueBigger - valueSmaller) < 0.01) {
        return 1;   //Return 1 if the values are similar
    }
    else {
        return 0;
    }
}

Since we're only interested on the truth value result here, we should use a boolean return to decide if the numbers are "similar". This could also be much more flexible and terse. We can use std::abs to achieve this.

//returns true if the variance is less than 0.01
bool valuesAreSimilar(double a, double b) {
  return abs(a - b) < 0.01;
}

//Function checks which value is larger
double max(double value01, double value02)
{
    if (value01 > value02) {
        return value01;
    }
    else if (value01 < value02) {
        return value02;
    }
    else {
        return 0;       //Return 0 if the values are equal!
    }
}

//Function checks which value is smaller
double min(double value01, double value02)
{
    if (value01 > value02) {
        return value02;
    }
    else if (value01 < value02) {
        return value01;
    }
    else {
        return 0;       //Return 0 if the values are equal!
    }
}

These are perhaps good to write for practice, but in a real code base, I recommend using std::min and std::max (credit to @MORTAL). For good recommendations in optimizing these two, check out @Greg Burghardt's answer. I'm changing the signatures to max() and min() for brevity's sake.

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (valueSmaller(value01, value02) != 0 || valueLarger(value01, value02) != 0) {
            cout << "The smaller value is: " << valueSmaller(value01, value02) << " , the larger value is: " << valueLarger(value01, value02) << "\n";
            if (similarValues(valueLarger(value01, value02), valueSmaller(value01, value02)) == 1) {
                cout<<"The numbers are almost equal. \n";
            }
        }
        else if (value01 == value02) {
            cout << "Both of these numbers are equal!" << "\n";
        }
        else {
            cout << "Not a value I know! Incorrect input" << "\n";
        }

    }
}

This one is a little wonky to look at. We can do a bit with it to make it nice by checking if the values are equal first. Another thing to note is that your final else statement can't be reached. If you'd like to do error checking, I recommend doing a stream failure check.

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (cin.fail()) {
            cout << "Not a value I know! Incorrect input" << "\n";
            continue;
        }
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
}

Your final code could look like this:

#include <iostream>
#include <algorithm>

using std::cout;
using std::cin;
using std::min;
using std::max;

//returns true if the variance is less than 0.01
bool valuesAreSimilar(double a, double b) {
  return std::abs(a - b) < 0.01;
}

int main()
{
    double value02; 
    double value01;

    while (cin >> value01 >> value02) {
        if (cin.fail()) {
            cout << "Not a value I know! Incorrect input" << "\n";
            continue;
        }
        if (value01 != value02) {
            cout << "The smaller value is: " << min(value01, value02) 
                << " , the larger value is: " << max(value01, value02) << "\n";
            if (valuesAreSimilar(value01, value02)) {
                cout << "The numbers are almost equal. \n";
            }
        }
        else {
            cout << "Both of these numbers are equal!" << "\n";
        }
    }
}