I could really use some help debugging my first function. I'm completely new to programming and this is my second day trying to learn C++ so please bear with me. I'm not sure if this is due to an error in syntax or an error in my understanding of how functions are used/called. Anyway here's my issue:
I was working through the drills in Programming: Principles and Practice and ended up working on a drill to enter a value (initially an integer, amended to double partway through the drill, hence the names) and a unit, and convert said value into meters which are then entered into a vector, sorted, and written to screen in order. My problem is that the only statement of the function that seems to work correctly is the "else" and statement (meters as well, since no conversion is necessary). If I enter cm, in, ft the value is not converted. It will accept those parameters, but for some reason it either does not amend the x value, or there is trouble somewhere further down in my code (such as it entering the unit_1 value into the vector before it is converted).
Anyway, any help would be greatly appreciated. Thanks
#include "std_lib_facilities.h"
double conversion(double x, string y)
{
if (y=="cm")
{
x*=100;
}
elseif (y=="ft")
{
x*=0.3048;
}
elseif (y=="m")
{
}
elseif (y=="in")
{
x*=(0.3048/12);
}
else
{
cout<<"Invalid unit. Please enter a valid unit:\n";
cin>>y;
conversion (x, y);
}
return 0;
}
int main()
{
cout<<"Enter an integer followed by a unit (cm, in, ft, m):\n";
double integer_1 = 0;
double integer_2 = 0;
vector<double> integers;
string unit_1;
while (cin>>integer_1>>unit_1)
{
conversion(integer_1, unit_1); //converts all units into meters
if (integer_1 == integer_2) //What to do if the values are equal
{
cout<<"The numbers are equal. Please enter new integers:\n";
}
if (integer_1 != integer_2) //what to do if the values are not equal
{
integers.push_back(integer_1); //adds value in meters to vector
sort(integers.begin(),integers.end()); //sorts vector
for (int i = 0; i<integers.size(); ++i) //displays list of vector entires
{
cout<<integers[i]<<"m ";
}
cout<<'\n';
if (abs(integer_1 - integer_2) < (1.0/10000000)) //informs user when numbers are close in value
{
cout<<"The numbers are almost equal.\n";
}
if (integer_1==integers[0]) //informs user when value entered is the smallest (after conversion)
{
cout<<"This is the smallest value so far\n";
}
integer_2=integer_1;
}
}
keep_window_open();
return 0;
}
Well I messed around with it and came up with something that does work (thanks to your advice), but it seems extremely ghetto. Are there any simple improvements I could make to this? I feel like setting integer_1=conversion(integer_1, unit_1) feels like something I shouldn't have to do... ie like I'm using a function wrong and putting a bandaid on it to make it work.
Any advice would be nice, and if the code suffices that works too I suppose! Thanks for the help
#include "std_lib_facilities.h"
double conversion(double x, string y)
{
double conversion_1=0;
if (y=="cm")
{
conversion_1=x/100;
}
elseif (y=="ft")
{
conversion_1=x*0.3048;
}
elseif (y=="m")
{
conversion_1=x;
}
elseif (y=="in")
{
conversion_1=x*(0.3048/12);
}
else
{
cout<<"Invalid unit. Please enter a valid unit:\n";
cin>>y;
conversion (x, y);
}
return conversion_1;
}
int main()
{
cout<<"Enter an integer followed by a unit (cm, in, ft, m):\n";
double integer_1 = 0;
double integer_2 = 0;
vector<double> integers;
string unit_1;
while (cin>>integer_1>>unit_1)
{
integer_1=conversion(integer_1, unit_1); //converts all units into meters
if (integer_1 == integer_2) //What to do if the values are equal
{
cout<<"The numbers are equal. Please enter new integers:\n";
}
if (integer_1 != integer_2) //what to do if the values are not equal
{
integers.push_back(integer_1); //adds value in meters to vector
sort(integers.begin(),integers.end()); //sorts vector
for (int i = 0; i<integers.size(); ++i) //displays list of vector entires
{
cout<<integers[i]<<"m ";
}
cout<<'\n';
if (abs(integer_1 - integer_2) < (1.0/10000000)) //informs user when numbers are close in value
{
cout<<"The numbers are almost equal.\n";
}
if (integer_1==integers[0]) //informs user when value entered is the smallest (after conversion)
{
cout<<"This is the smallest value so far\n";
}
integer_2=integer_1;
}
}
keep_window_open();
return 0;
}
First pass: This code should be functionally equivalent to what you have. All I have done is put in some whitespace and changed the names of variables and functions to make the code more readable. Doing it this way makes some comments redundant, and if the code reads just as well without a comment, that's okay!
Also, on line 44 I replaced the other if(...) with a simple else, since it is logically equivalent to what you were doing before.
double ConvertToMeters(double value, string units)
{
double valueInMeters = 0;
if (units == "cm")
{
valueInMeters = value/100;
}
elseif (units == "ft")
{
valueInMeters = value*0.3048;
}
elseif (units == "m")
{
valueInMeters = value;
}
elseif (units == "in")
{
valueInMeters = value*(0.3048/12);
}
else
{
string newUnits;
cout << "Invalid unit. Please enter a valid unit:\n";
cin >> newUnits;
ConvertToMeters(value, newUnits);
}
return valueInMeters;
}
int main()
{
cout<<"Enter an integer followed by a unit (cm, in, ft, m):\n";
double mostRecentValue = 0;
double penultimateValue = 0;
vector<double> valuesInMeters;
string units;
while (cin >> mostRecentValue >> units)
{
double mostRecentValueInMeters = ConvertToMeters(mostRecentValue, units); //converts all units into meters
if (mostRecentValueInMeters == penultimateValue) //What to do if the values are equal
{
cout<<"The numbers are equal. Please enter new integers:\n";
}
else //what to do if the values are not equal
{
valuesInMeters.push_back(mostRecentValueInMeters); //adds value in meters to vector
sort(valuesInMeters.begin(),valuesInMeters.end()); //sorts vector
for (int i = 0; i<valuesInMeters.size(); ++i) //displays list of vector entires
{
cout<<valuesInMeters[i]<<"m ";
}
cout<<'\n';
if (abs(mostRecentValue - penultimateValue) < (1.0/10000000)) //informs user when numbers are close in value
{
cout<<"The numbers are almost equal.\n";
}
if (mostRecentValue==valuesInMeters[0]) //informs user when value entered is the smallest (after conversion)
{
cout<<"This is the smallest value so far\n";
}
penultimateValue = mostRecentValue;
}
}
keep_window_open();
return 0;
}
EDIT: I had erroneously declared mostRecentValueInMeters an int on line 39
Next: I'd try to get rid of that recursive call inside of the conversion function. Instead of trying to do conversion over and over again, I'd rather just get a valid unit first, THEN attempt the conversion once, without having to call conversion recursively. Also, this fixes a bug in the program where the value being returned if you used the wrong units would always be zero. To do this, I'll create another function and use it with the conversion function.
Thanks so much for all of the help. I really like the check to see if a valid string was entered and will definitely use it in the future. I had solved the problem of it spitting out a 0 in the else case prior to rechecking this post, but it certainly wasn't as simple as how you have coded it.
