The purpose of this program is to write a code that will give a user a list or menu of choices that show colors and their values according to some data and prompt them to enter input to determine the value of a resistor. I have completed my source code for this program and it runs perfectly, but I would like some feedback on how to improve this code before I submit it.
enter image description here
The picture above shows the colors and the data values associated with each.
My professor has given me the liberty to use either 1 or 2 dimensional arrays. I chose 1 dimensional arrays.
#include <iostream>
#include <cmath>
#include <string>
using namespace std;
//Function prototypes
int color(string[], const int);
double time(string[], double[], const int, int, int, int);
int tolerance(string[], double[], const int);
int main()
{
// Variables
const int band = 10, multiplier = 12, toleranceSize = 4; // Represents size of each string array
int a, b, c; // Holds the 3 colors from band which are entered by user
int output; // Contains tolerance
double product; // Stores result from calculation
// Defined string arrays
string BAND_COLOR_CODE[band] = { "black", "brown", "red", "orange",
"yellow", "green", "blue", "violet",
"grey", "white" };
string MULTIPLIER_COLOR_CODE[multiplier] = { "black", "brown", "red", "orange",
"yellow", "green", "blue", "violet",
"grey", "white", "gold", "silver" };
string TOLERANCE_COLOR_CODE[toleranceSize] = { "brown", "red", "gold", "silver" };
// Arrays which hold numeric values for the string arrays
double multiplierArray[multiplier] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0.1, 0.01 };
double toleranceArray[toleranceSize] = { 1, 2, 5, 10 };
// Loop to show each color and the number it corresponds to
for (int i = 0; i < band; i++)
{
cout << BAND_COLOR_CODE[i] << " == " << multiplierArray[i] << endl;
}
// Call the function three times and store the information (i.e.: color and its value) in the variables a, b, and c
a = color(BAND_COLOR_CODE, band);
b = color(BAND_COLOR_CODE, band);
c = color(BAND_COLOR_CODE, band);
cout << "\n";
//Call the function to store the result from the calculation
product = time(MULTIPLIER_COLOR_CODE, multiplierArray, multiplier, a, b, c);
cout << "\n";
//Call the function to store the tolerance
output = tolerance(TOLERANCE_COLOR_CODE, toleranceArray, toleranceSize);
cout << "\n";
//Display information about the resistor
cout << "This resistor has " << product << " ohms with " << output << " % tolerance." << endl;
cout << "\n";
system("PAUSE");
return 0;
}
/**
* Pre-Condition: This function accepts a string array and its size. *
* Post-Condition: It sets each color to a particular numeric value *
* and converts it into an int to return after accepting user input. *
*/
int color(string BAND_COLOR_CODE[], const int band)
{
// Variables
char code[10]; // Represents the colors from the string array BAND_COLOR_CODE
int num = 0; // Holds numeric value from 0-9 for color band
static int j = 0; // Stores user input for color
++j; // Increments to allow user to input colors in succession
cout << "\n";
// Prompt for user input
cout << "Enter a color for band " << j << ": > ";
cin.getline(code, 10);
// Loop to take care of the case of letters
for (int i = 0; i < 10; i++)
code[i] = tolower(code[i]);
// Loop to set user input to a number
for (int i = 0; i < band; i++)
{
if(code == BAND_COLOR_CODE[i])
{
switch (i)
{
case 0:
{
num = 0;
break;
}
case 1:
{
num = 1;
break;
}
case 2:
{
num = 2;
break;
}
case 3:
{
num = 3;
break;
}
case 4:
{
num = 4;
break;
}
case 5:
{
num = 5;
break;
}
case 6:
{
num = 6;
break;
}
case 7:
{
num = 7;
break;
}
case 8:
{
num = 8;
break;
}
case 9:
{
num = 9;
break;
}
default:
{
cout << "ERROR!" << endl;
}
}
}
}
return num;
}
/**
* Pre-Condition: This function accepts two arrays, their size, and three *
* variables that will hold colors. *
* Post-Condition: It accepts user input and converts it into a int to *
* use in data calculation. It returns a product. *
*/
double time(string MULTIPLIER_COLOR_CODE[], double multiplierArray[], const int multiplier, int a, int b, int c)
{
// Variables
char code[10]; // Represents the colors from the string array BAND_COLOR_CODE
double total = 0; // Overall sum of a, b, and c
double num = 0; // Holds numeric values for multiplier
double value; // Stores value of resistor
// Loop to show colors in the multiplier
for (int i = 0; i < multiplier; i++)
{
cout << MULTIPLIER_COLOR_CODE[i] << " == " << multiplierArray[i] << endl;
}
cout << "\n";
// Prompt for user input
cout << "Enter a color for the multiplier: > ";
cin.getline(code, 10);
// Loop to take care of case for letters
for (int i = 0; i < 10; i++)
code[i] = tolower(code[i]);
// Loop to set user input to a number
for (int i = 0; i < multiplier; i++)
{
if (code == MULTIPLIER_COLOR_CODE[i])
{
switch (i)
{
case 0:
{
num = 0;
break;
}
case 1:
{
num = 1;
break;
}
case 2:
{
num = 2;
break;
}
case 3:
{
num = 3;
break;
}
case 4:
{
num = 4;
break;
}
case 5:
{
num = 5;
break;
}
case 6:
{
num = 6;
break;
}
case 7:
{
num = 7;
break;
}
case 8:
{
num = 8;
break;
}
case 9:
{
num = 9;
break;
}
case 10:
{
num = 0.1;
break;
}
case 11:
{
num = 0.01;
break;
}
default:
{
cout << "ERROR!" << endl;
}
}
}
}
total += (a * 100) + (b * 10) + c;
value = total * pow(10, num);
return value;
}
/**
* Pre-Condition: This function accepts two arrays and their size. *
* Post-Condition: It gets user input and converts it into an int *
* to be returned. *
*/
int tolerance(string TOLERANCE_COLOR_CODE[], double toleranceArray[], const int toleranceSize)
{
// Variables
char code[10]; // Represents the colors from the string array BAND_COLOR_CODE
int num = 0; // Holds numeric values for tolerance
// Loop to set user input to a number
for (int i = 0; i < toleranceSize; i++)
{
cout << TOLERANCE_COLOR_CODE[i] << " == " << toleranceArray[i] << endl;
}
cout << "\n";
// Prompt for user input
cout << "Enter a color for the tolerance: > ";
cin.getline(code, 10);
// Loop to take care of case for letters
for (int i = 0; i < 10; i++)
code[i] = tolower(code[i]);
// Loop to set user input to a number
for (int i = 0; i < toleranceSize; i++)
{
if (code == TOLERANCE_COLOR_CODE[i])
{
switch (i)
{
case 0:
{
num = 1;
break;
}
case 1:
{
num = 2;
break;
}
case 2:
{
num = 5;
break;
}
case 3:
{
num = 10;
break;
}
default:
{
cout << "ERROR!" << endl;
}
}
}
}
return num;
}
1 Answer 1
I have found a couple of things that could help you improve your code.
Don't abuse using namespace std
Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. It's not necessarily wrong to use it, but you should be aware of when not to (as when writing code that will be in a header).
Don't use system("cls")
There are two reasons not to use system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++.
Combine related data
The color strings and the values they represent are tightly bound but not in your data structures. Consider instead defining an object to contain both a color name and the associated value. That way it's much more clear that they are related and helps in both maintenance and understanding of the code.
Use a menu object or at least a common menu function
In a number of places in your code, you have something like a menu. Your code presents a prompt (a list of values) and then asks the user to pick one. Rather than repeating that code in many places, it would make sense to make it generic. Only the prompt strings actually change, but the underlying logic of presenting the choices and asking for input are all the same. It looks like you're a beginning programmer, and so perhaps you haven't learned about objects yet, but this kind of repeated task with associated data is really well-suited to object-oriented programming and that's something that C++ is very good at expressing.
Use const where practical
Your string labels are not and should not be altered by the program, so they should be declared const. In general, whenever you are writing a variable or function prototype look for places you can use const.
Localize data to where it is used
The MULTIPLIER_COLOR_CODE and related strings are never actually used in main except to pass to other routines which suggests that they should be moved to those routines instead. Make them static const and move them where they belong.
Think of the user of your code
Not all resistors have five bands. In fact, in practice most only have four. You might want to consider that and allow the user to tell the program how many bands are on the resistor.
It might also be a nice feature to allow an uninformed user to still get the right value even if they enter the bands in reverse order.