Resistor Program Using Functional Decomposition

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.

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
#include
#include
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] =

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.

StackExchange Code Review Q#87240, answer score: 4