views:

126

answers:

4
#include <iostream>
using namespace std;

int main(){
    int findMax(int *);

    const int MAX = 100;
    int values[MAX];
    char ivals[256];
    // Get the space-separated values from user input.
    cin.getline(ivals, 256, '0');
    char *helper;
    // Clean input array and transfer it to values.
    for(int i = 0; i < (MAX) && ivals[i] != 0; i++){
        helper = ivals[i * 2];
            values[i] = atoi(helper);

    }

    int mval = findMax(values);
    cout << values << endl << mval;
    return 0;
}
//Function to find the maximum value in the array
int findMax(int arr[]){
    int localmax = 0;
    for(int i = 0; i < (sizeof(arr)/sizeof(int)); i++){
        if(arr[i] > localmax){
            localmax = arr[i];
        }
    }
    return localmax;
}

The purpose of this program is for the user to input a space-separated series of values ended by a 0. That array is then to be analyzed to find the max. I figured out how to convert what is originally a char[] into an int[] so that I can use the findMax() function on it without error but the sorting loop seems to have a problem of its own and when "cout << values << endl << mval;" is called, it returns only a memory address instead of what should be a non-spaced sequence of ints. Can anybody explain what I am doing wrong? It seems that I may have made some mistake using the pointers but I cannot figure out what.

+2  A: 

Array of int is promoted to a pointer to int when passed to a function. There is no operator << taking ordinary array. If you want to use operator << this way, you need to use std::vector instead.

Note: it is possible technically to distinguish array when passed to a function using template, but this is not implemented for standard operator <<.

Suma
+7  A: 

Printing values won't print the contents of the array as you expect, it will print the memory location of the first element of the array.

Try something like this instead:

#include <iterator>
#include <algorithm>

// ...

copy(&values[0], &values[MAX], ostream_iterator(cout, " "));

Sorry I can't post actual working code, but your original post is a mess with many syntax and syntactic errors.

EDIT: In the interest of being more complete and more approachable & understandable to beginners, I've written a small program that illustrates 4 ways to accomplish this.

Method 1 uses copy with an ostream_iterator as I've done above. Method 2 below is probably the most basic & easiest to understand. Method 3 is a C++0x method. I know the question is tagged C++, but I thought it might be educational to add this. Method 4 is a C++ approach using a vector and for_each. I've implemented a functor that does the dumping.

Share & Enjoy

#include <iostream>
#include <iterator>
#include <algorithm>
#include <functional>
#include <vector>
using namespace std;

struct dump_val : public unary_function<int,void>
{
    void operator()(int val)
    {
        cout << val << " ";
    }
};

int main(){
    int vals[5] = {1,2,3,4,5};


    // version 1, using std::copy and ostream_iterator
    copy(&vals[0], &vals[5], ostream_iterator<int>(cout, " "));
    cout << endl;

    // version 2, using a simple hand-written loop
    for( size_t i = 0; i < 5; ++i )
        cout << vals[i] << " ";
    cout << endl;

    // version 3, using C++0x lambdas
    for_each(&vals[0], &vals[5], [](int val) 
    {
        cout << val << " ";
    }
    );
    cout << endl;

    // version 4, with elements in a vector and calling a functor from for_each
    vector<int> vals_vec;
    vals_vec.push_back(1);
    vals_vec.push_back(2);
    vals_vec.push_back(3);
    vals_vec.push_back(4);
    vals_vec.push_back(5);
    for_each( vals_vec.begin(), vals_vec.end(), dump_val() );
    cout << endl;

}
John Dibling
+1 For using the standard library, but this is probably not the simplest approach for a beginner :)
luke
John Dibling
+2  A: 
for(int i = 0; i < (sizeof(arr)/sizeof(int)); i++){

sizeof(arr) here is the size of the pointer to the array. C++ will not pass the actual array, that would be grossly inefficient. You'd typically only get one pass through the loop. Declare your function like this:

int findMax(int* arr, size_t elements) {
    //...
}

But, really, use a vector.

Oh, hang on, the question. Loop through the array and print each individual element.

Hans Passant
+3  A: 

When you pass around an array of X it's really a pointer to an array of X that you're passing around. So when you pass values to cout it only has the pointer to print out.

You really should look into using some of the standard algorithms to make your life simpler.

For example to print all the elements in an array you can just write

std::copy(values, values+MAX, std::ostream_iterator<int>(std::cout, "\n"));

To find the max element you could just write

int mval = *std::max_element(values, values+MAX);

So your code becomes

#include <iostream>
using namespace std;

int main(){

    const int MAX = 100;
    int values[MAX];
    char ivals[256];
    // Get the space-separated values from user input.
    cin.getline(ivals, 256, '0');
    char *helper;
    // Clean input array and transfer it to values.
    for(int i = 0; i < (MAX) && ivals[i] != 0; i++){
        helper = ivals[i * 2];
            values[i] = atoi(helper);

    }

    copy(values, values+MAX, ostream_iterator<int>(cout, "\n"));
    cout << *std::max_element(values, values+MAX);
    return 0;
}

Doing this removes the need for your findMax method altogether.

I'd also re-write your code so that you use a vector instead of an array. This makes your code even shorter. And you can use stringstream to convert strings to numbers.

Something like this should work and is a lot less code than the original.

int main(){


    vector<int> values;
    char ivals[256];

    // Get the space-separated values from user input.
    cin.getline(ivals, 256, '0');

    int temp = 0;
    stringstream ss(ivals);
    //read the next int out of the stream and put it in temp
    while(ss >> temp) {
        //add temp to the vector of ints
        values.push_back(temp);
    }

    copy(values.begin(), values.end(), ostream_iterator<int>(cout, "\n"));
    cout << *std::max_element(values.begin(), values.end());
    return 0;
}
Glen
+1 for `std::max_element` -- that's how this code should be written in the first place anyway.
Billy ONeal