0
#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.

DaveStance
  • 421
  • 1
  • 4
  • 17
  • Related : http://stackoverflow.com/questions/2037736/finding-size-of-int-array/ and http://stackoverflow.com/questions/1975128/sizeof-an-array-in-the-c-programming-language/ – Prasoon Saurav Sep 09 '10 at 16:59
  • 1
    @aaa carp: once again: an array is not a pointer, it has a different type, size... the language performs automatic conversions from arrays to the address of the first element in the array, but that is it. Saying that an array is a pointer is like saying that an int is a double just because `int i = 10; double d = i;` compiles. – David Rodríguez - dribeas Sep 10 '10 at 09:01

4 Answers4

8

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
  • 99,718
  • 31
  • 186
  • 324
  • 3
    +1 For using the standard library, but this is probably not the simplest approach for a beginner :) – luke Sep 09 '10 at 17:00
  • 5
    Depends on your definition of "simple." :) Personally I generally find it easier & simpler to use STL's algorithms than crafting a hand-written loop. I do acknowledge that a lot of C++ programmers don't realize `copy` can be used in this way, however. – John Dibling Sep 09 '10 at 17:02
3

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
  • 21,816
  • 3
  • 61
  • 76
2

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
  • 33,181
  • 16
  • 123
  • 191
2
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
  • 922,412
  • 146
  • 1,693
  • 2,536