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

Ohm's Law calculator

Submitted by: @import:stackexchange-codereview··
0
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 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.