I'm trying to create a program that determines the min when I press 0 and the max when I press 9. The numbers it will pull from is an array of 5 numbers. Can you guys help me understand what's wrong?
Here's my code:
int Lab2Numbers[5] = {0, 5, 4, 2, 7};
void setup() {
Serial.begin(9600);
}
void loop() {
int nMax = 0;
int nMin = 0; // Declaring and identifying variables
for (int x = 0; x <= 2; x++) // Comparing all the numbers that are less than or equal to 2
{
if (nMax <= Lab2Numbers[x])
{
nMax = Lab2Numbers[x];
}
}
for (int x = 0; x <= 2; x++)
{
if (nMin >= Lab2Numbers[x])
{
nMin = Lab2Numbers[x];
}
}
if (Serial.available()) // To check if any number/char typed in Serial Monitor
{
char UserInput = Serial.read(); // Read in a typed number from Serial Monitor
if (UserInput == '0' || UserInput == '9') // is UserInput a number?
{
if (UserInput == '0') // when ‘0’ is typed
{
Serial.print("The Minimun number is: ");
Serial.println(nMin);
}
else // when ‘9’ is typed
{
Serial.print("The Maximum number is: ");
Serial.println(nMax);
}
}
else
{
Serial.println("Invalid answer. Please input the number 9 or 0: ");
}
}
}
int Lab2Numbers[5] = {0, 5, 4, 2, 7};
void setup() {
Serial.begin(9600);
}
void loop() {
int nMax = -32767;
int nMin = 32767; // Declaring and identifying variables
for (int x = 0; x <= 5; x++) // Comparing all the numbers that are less than or equal to 2
{
if (nMax <= Lab2Numbers[x])
{
nMax = Lab2Numbers[x];
}
}
for (int x = 0; x <= 5; x++)
{
if (nMin >= Lab2Numbers[x])
{
nMin = Lab2Numbers[x];
}
}
if (Serial.available()) // To check if any number/char typed in Serial Monitor
{
char UserInput = Serial.read(); // Read in a typed number from Serial Monitor
if (UserInput == '0' || UserInput == '9') // is UserInput a number?
{
if (UserInput == '0') // when ‘0’ is typed
{
Serial.print("The Minimun number is: ");
Serial.println(nMin);
}
else // when ‘9’ is typed
{
Serial.print("The Maximum number is: ");
Serial.println(nMax);
}
}
else
{
Serial.println("Invalid answer. Please input the number 9 or 0: ");
}
}
}
int nMax = -32767;
int nMin = 32767;
for (int x = 0; x < 5; x++) {
if (nMax < Lab2Numbers[x]) {
nMax = Lab2Numbers[x];
}
}
for (int x = 0; x < 5; x++) {
if (nMin > Lab2Numbers[x]) {
nMin = Lab2Numbers[x];
}
}
the definition of the Lab2 array without explicit dimension
the define for the number of elements (so you can just add values to the array and don't have to change code elsewhere)
the initialization of the min and max values (the negative value is still one to big to be perfect)
you have to take care of the CR and LF that separate lines
I like to have the opening '{' on the same line as if's and for's, but thats personal taste.
I think
if (myValue > nMax) { // easier to understand/read (for me
)
if (nMax < myValue) { // than this
if myValue is bigger than Max then...
if Max is less than myValue then...
I associate Max with big(ger), maybe that's the reason.
@vbextreme: Why set the min and max to element 0? They should be set as shown earlier:
int nMax = -32767;
int nMin = 32767;
Also, the for loop
for (int x = 1; x < 5; x++) {
if (nMax < Lab2Numbers[x]) {
nMax = Lab2Numbers[x];
}
if (nMin > Lab2Numbers[x]) {
nMin = Lab2Numbers[x];
}
}
should be written as:
// #define ELMS (int)sizeof(Lab2Numbers)/sizeof(int) // Only works on int arrays. Use instead:
#define ELMS(x) (sizeof(x) / sizeof(x[0])) // Works on all native arrays
// other code...
for (int x = 0; x < ELMS(Lab2Numbers); x++) {
if (nMax <= Lab2Numbers[x]) {
nMax = Lab2Numbers[x];
}
if (nMin >= Lab2Numbers[x]) {
nMin = Lab2Numbers[x];
}
}
it set the minimum and maximum value to a unreal when you can set the first element of the array?
In this way the code in addition to being more powerful is especially more portable because it does not depend on the size of the int type.
I never liked the sizeof/sizeof.
it set the minimum and maximum value to a unreal when you can set the first element of the array?
Ok, but a common problem is that the contents of the array are read from some unknown source so there actually is no first element known at compile time:
int Lab2Numbers[10];
In your example, you can set it to the first element, but if you know the content of the array at compile time, why are you even bothering to search the list in code? Just look through the list and assign it.
In this way the code in addition to being more powerful is especially more portable because it does not depend on the size of the int type.
More powerful? If the array is initialized when it is defined, how does an unnecessary march through the array make it more powerful? Just look at the data and set it. If you don't know the data values at compile time, then use symbolic constants. Since you know the data type of the array (e.g., an int array here), there are symbolic constants available in limits.h for basic data type, so you could define nMax with INT_MIN and nMin with INT_MAX before searching the array.
I never liked the sizeof/sizeof.
Why not? It's a lot more flexible than hardcoding the loop values like you have. Also, the #define I presented works with any array type.
why would a vector declared at compile time?
Performance:
you assign two values unnecessary to min and max and then lose at least 2 instructions unnecessarily.
These two values to the first loop will most likely be replaced because they are the maximum and minimum values of type int, so performing unnecessary for index of 0 to guess that costs about 10 instructions
Total 12 instruction is more slow of 2.
If you not have a 0 index is simple to return error! not have a min or max!
if you have only One data in vector the for exit immediately.
sizeof / sizeof not use it because I do not like and have never found, but this is just my opinion