Thread: Stuck at arrays

Hey all,

I purchased the book 'coz I'm an eager learner of C++.

But I got stuck at arrays. Even though I think I understand how array works when I got to solve problems I got stuck on seemingly simple problem.

The problem is as follows:

Write a program that takes in 50 numbers and prints out the highest, the lowest, the average and then all 50 numbers, one per line.

This is my code:

Code:

#include <iostream>
#include <ctime>
#include <cstdlib>


int size;
int theLowestNumber(int array[],int size) {
    int index_of_smallest_value;
    for (int i = 0; i < size; i++) {
        if (array[i] < array[index_of_smallest_value]) {
            index_of_smallest_value = i;
        }
    }
    std::cout << index_of_smallest_value;
}


int theGreatestNumber(int array[], int size) {


    int index_of_greatest_value;
    for (int i = 0; i < size; i++) {


        if (array[i] > i) {
            i = array[i];
            index_of_greatest_value = i;
        }
    }
    std::cout << index_of_greatest_value;
}


void displayArray (int array[], int size) {


    std::cout << "{";
    for (int i = 0; i < size; i++) {


        if (i != 0) {
            std::cout << ", ";
    }
    std::cout << array[i];
    }
    std::cout << "}";
}




int main() {


    int array[10];
    srand(time(NULL));
    for (int i = 0; i < 10; i++) {


        array[i] = rand() % 100;
    }




    theGreatestNumber(array, 10);
    std::cout << "\n";
    theLowestNumber(array, 10);
    std::cout << "\n";
    std::cout << "Array: ";
    displayArray(array, 10);
    std::cout << "\n";


}

The output from

Code:

theGreatestNumber()

is giving me only the first number from the array despite clearly looping through the whole array and the

Code:

theLowestNumber()

function call is giving me a number that's not even in the array. Bizarre.

I know this isn't telling anything good about my problem-solving skills ..... hopefully I'll get better with time.

The first thing I see is that you're using a couple of variables that are not initialized, you need to insure that all of your variables are initialized before you try to use them.

Next what exactly is your program outputting?

After initializing all the variables this is the output I get when I run the program.


83
3

Array: {83, 86, 77, 15, 93, 35, 86, 92, 49, 21}


The first number is incorrect, but the second number looks correct. Remember you're printing the "index" of the lowest number not the lowest number. It would probably help you if you printed a little more information than just the number something like this perhaps:

Code:

    std::cout << "Index of smallest value: " << index_of_smallest_value << " which is: " << array[index_of_smallest_value] << std::endl;

Your "greatest()" function should look very similar to the lowest function when you're finished.

Now to some other serious problems. You have a global variable that you should delete and you promised that you would return values from your functions, so you must return values from those functions.

You probably should be using a named constant for the size of your array so that when you get ready to change the size of the array to the size required by the assignment you'll only need to change the value in one place.

Jim

Thanks so much for chiming in.

Well I moved things around a bit and this is what I've got:

Code:

#include <iostream>
#include <ctime>
#include <cstdlib>


int size = 10;


int theLowestNumber(int array[],int size) {
    int index_of_smallest_value;
    for (int i = 0; i < size; i++) {
        if (array[i] < array[index_of_smallest_value]) {
            index_of_smallest_value = i;
        }
    }
    std::cout << index_of_smallest_value;
}


int theGreatestNumber(int array[], int size) {


    int index_of_greatest_value;
    for (int i = 0; i < size; i++) {


        if (array[i] > array[index_of_greatest_value]) {
                index_of_greatest_value = i;
        }
    }
    std::cout << index_of_greatest_value;
}








void displayArray (int array[], int size) {


    std::cout << "{";
    for (int i = 0; i < size; i++) {


        if (i != 0) {
            std::cout << ", ";
    }
    std::cout << array[i];
    }
    std::cout << "}";
}




int main() {


    int array[size];
    srand(time(NULL));
    for (int i = 0; i < size; i++) {


        array[i] = rand() % 100;
    }




    theGreatestNumber(array, size);
    std::cout << "\n";
    theLowestNumber(array, size);
    std::cout << "\n";
    std::cout << "Array: ";
    displayArray(array, 10);
    std::cout << "\n";


}

Speaking of the global variable. It's there on purpose 'coz if I don't define it at the top function definitions that are below it will scream about variable not being declared and having it declared in the main function doesn't cut it.

Btw the code I just shared spits out "Segmentation fault. Core dumped". After googling for a minute I learned that it's because I'm trying to access memory I don't have access to. I hope I'm getting closer to solution. Little bit embarrassing that I struggle to make this trivial problem solved.

Speaking of the global variable. It's there on purpose 'coz if I don't define it at the top function definitions that are below it will scream about variable not being declared and having it declared in the main function doesn't cut it.

Well since you're passing size into the function as a parameter with the same name the global is not used in those functions.

But most importantly you're trying to use that global to "size" your array and since that global is not a compile time constant your program should fail to compile:

So you really should just make that variable local to main() as a const qualified variable.

Here is what my compiler reports about your program:

main.cpp||In function ‘int theLowestNumber(int*, int)’:|
main.cpp|9|warning: declaration of ‘size’ shadows a global declaration [-Wshadow]|
main.cpp|6|note: shadowed declaration is here|
main.cpp|17|warning: no return statement in function returning non-void [-Wreturn-type]|
main.cpp||In function ‘int theGreatestNumber(int*, int)’:|
main.cpp|20|warning: declaration of ‘size’ shadows a global declaration [-Wshadow]|
main.cpp|6|note: shadowed declaration is here|
main.cpp|32|warning: no return statement in function returning non-void [-Wreturn-type]|
main.cpp||In function ‘void displayArray(int*, int)’:|
main.cpp|41|warning: declaration of ‘size’ shadows a global declaration [-Wshadow]|
main.cpp|6|note: shadowed declaration is here|
main.cpp||In function ‘int main()’:|
main.cpp|62|error: ISO C++ forbids variable length array ‘array’ [-Wvla]|

So you really should just make that variable local to main() as a const qualified variable.

Btw the code I just shared spits out "Segmentation fault. Core dumped".

It's probably being caused because you failed to implement some of the suggested changes I told you about in my first post. You're still not initializing all of your variables before you try to use them and you need to either return a value from your "lowest" and "greatest" functions or tell the compiler you won't be returning any values by using "void" for the return type.

Jim

Thanks for your patience.

This is my latest code update. Still tweaking it ...

Code:

#include <iostream>#include <ctime>
#include <cstdlib>






int theLowestNumber(int array[], int size) {
    int index_of_smallest_value;
    for (int i = 0; i < size; i++) {
        if (array[i] < array[index_of_smallest_value]) {
            index_of_smallest_value = i;
        }
    }
    return index_of_smallest_value;
}


int theGreatestNumber(int array[], int size) {


    int index_of_greatest_value;
    for (int i = 0; i < size; i++) {


        if (array[i] > array[index_of_greatest_value]) {
                index_of_greatest_value = i;
        }
    }
    return index_of_greatest_value;
}








void displayArray (int array[], int size) {


    std::cout << "{";
    for (int i = 0; i < size; i++) {


        if (i != 0) {
            std::cout << ", ";
    }
    std::cout << array[i];
    }
    std::cout << "}";
}




int main() {


    const int size = 10;
    int array[10];
    srand(time(NULL));
    for (int i = 0; i < size; i++) {


        array[i] = rand() % 100;
    }




    std::cout << theGreatestNumber(array, size);
    std::cout << "\n";
    std::cout << theLowestNumber(array, size);
    std::cout << "\n";
    std::cout << "Array: ";
    displayArray(array, size);
    std::cout << "\n";

}

values are being returned instead of being printed out and moved the global variable to the main function and turned it into a constant. Still getting core dumped. Will keep working on that. Will keep you posted.

You're still not initializing all of your variables!

Code:

int theGreatestNumber(int array[], int size) {
    int index_of_greatest_value;   // THIS IS NOT INITIALIZED (and this is not the only variable not initialized)!!
...

And don't forget that you're printing out the "index" not the value of the element of the array.


Jim

I had to redo the whole thing. Even when I initialised all the variables that I successfully didn't bother to initialise the code stopped making sense to me.
So I started over.

This is what I've got now. It's little bit clearer and easier to read:

Code:

#include <iostream>#include <ctime>
#include <cstdlib>






int theLowestNumber(int array[], int size) {


    for (int i = 0; i < size; i++) {
        if (array[i] < i) {
            i = array[i];
        }
        return i;
    }


}


int theGreatestNumber(int array[], int size) {




    for (int i = 0; i < size; i++) {


        if (array[i] > i) {
                i = array[i];
        }
        return i;
    }


}








void displayArray (int array[], int size) {


    std::cout << "{";
    for (int i = 0; i < size; i++) {


        if (i != 0) {
            std::cout << ", ";
    }
    std::cout << array[i];
    }
    std::cout << "}";
}




int main() {


    const int size = 10;
    int array[10];
    srand(time(NULL));
    for (int i = 0; i < size; i++) {


        array[i] = rand() % 100;
    }




    std::cout << theGreatestNumber(array, size);
    std::cout << "\n";
    std::cout << theLowestNumber(array, size);
    std::cout << "\n";
    std::cout << "Array: ";
    displayArray(array, size);
    std::cout << "\n";


}

The only problem is that output from theGreatestNumber is giving me the first number (not an index) from the array and theLowestNumber is giving me 0. Even if it's an index it's wrong 'coz the first number in the array isn't the lowest value. As you can see the only difference between those two is the name and I flipped the comparison operator which I thought will do the trick.

Only the display function works the way it supposed to.

and I keep on thinking ......

It seems you are using a pretty old book. Many so-called C++ books or courses are based on K & R C, changing files with streams, but still using C-style arrays and pointers and ...

Even in the old C++98 you could do the task like this:

Code:

#include <iostream>
#include <vector>
#include <algorithm>

using namespace std;

int main()
{
  std::vector<int> numbers = { 5, 7, 3, 6 };

  std::cout << "\nmin value: " << *std::min_element(numbers.begin(), numbers.end());

  std::cout << "\nmax value: " << *std::max_element(numbers.begin(), numbers.end());

  return 0;
}

OUTPUT
=====
min value: 3
max value: 7

The exercise says one function returns the highest value and the second returns the lowest.

Ehm ... Old book ? Perhaps. First printing 2013. I'm working with Alex Allain's book advertised here. I like that book. Very informative and I've learned a lot. I've solved all the previous challenges but this one keeps me stuck. I keep searching for what I'm missing.

Doesn't sound so old. Just wonder why it doesn'y teach then modern stuff. Anyway, finding the max of an array is not difficult. Normally you assume the first value is the max. Then you loop from the second elem to end. If an elem is greater than orig max then set max to the new elem.

Code:

int theGreatestNumber(int array[], int size)
{
  assert(size > 0); // #include <cassert>
  int max = array[0];
  for (int i = 1; i < size; i++)
  {
    if (array[i] > max)
    {
      max = array[i];
    }
    return max;
  }
}

Doesn't sound so old.

Just because a book has been fairly recently published doesn't mean that the book is using "modern C++". From what I've seen of the book it is teaching C-style C++ before it goes into things like vector, string, etc.

By the way @OldGuy2 how may times do you think that loop will run?

Jim

I feel like I need more time with this. For some reason my primitive brain don't understand it. That's why I'm stuck. Thank you all for trying to educate me. Greatly appreciated !