patterncppMinor
Ohm's Law calculator
Viewed 0 times
ohmcalculatorlaw
Problem
I'm looking for ways to make this calculator more efficient and/or make the code a little easier to read.
#include
#include
using namespace std;
// variables to be used in the program
double result, valueA, valueB, valA, valB;
int choice;
char cont;
//functions to perform the calculations
double mult(double valueA, double valueB) {
result = valueA * valueB;
return result;
}
double div(double valueA, double valueB) {
result = valueA / valueB;
return result;
}
// functions that prompt the user for values to be calculated
double getValueA(string unit) {
cout > valA;
cout > valB;
cout > choice;
switch(choice) {
case 0:
exit(0);
case 1:
getValueA("Amps");
getValueB("Ohms");
mult(valA, valB);
displayResult("Volts");
break;
case 2:
getValueA("Volts");
getValueB("Amps");
div(valA, valB);
displayResult("Ohms");
break;
case 3:
getValueA("Volts");
getValueB("Ohms");
div(valA, valB);
displayResult("Amps");
break;
case 4:
getValueA("Volts");
getValueB("Amps");
mult(valA, valB);
displayResult("Watts");
break;
case 5:
getValueA("Watts");
getValueB("Amps");
div(valA, valB);
displayResult("Volts");
break;
case 6:
getValueA("Watts");
getValueB("Volts");
div(valA, valB);
displayResult("Amps");
break;
default:
cout > cont;
} while(cont != 'n');
return 0;
}Solution
-
Indent your code.
-
There is no need to define functions for multiplication and division. It is just as clear to say
-
-
Don't use
-
Don't use a global for the result. Pass it as a parameter to
-
You can, and should, avoid global variables completely here, as
-
If the user types "no" or "N" it is treated as Yes. Handle this more carefully. For example, consider only the lowercase of the first letter.
Indent your code.
-
There is no need to define functions for multiplication and division. It is just as clear to say
a * b as mult(a,b).-
getValueA and getValueB do exactly the same thing, except with a different global. The return value of these functions is ignored. It would be better to merge these into one and use the return value.-
Don't use
valA and valB for variable names. Use local variables with names like volt. -
Don't use a global for the result. Pass it as a parameter to
displayResult, like this:case 1:
double amps = getValue("Amps");
double ohms = getValue("Ohms");
displayResult(amps * ohms, "volts");-
You can, and should, avoid global variables completely here, as
valueA and valueB are never used in the global context, they can be removed.-
If the user types "no" or "N" it is treated as Yes. Handle this more carefully. For example, consider only the lowercase of the first letter.
Code Snippets
case 1:
double amps = getValue("Amps");
double ohms = getValue("Ohms");
displayResult(amps * ohms, "volts");Context
StackExchange Code Review Q#138705, answer score: 6
Revisions (0)
No revisions yet.