iterators problem

[gong]

hi

I am trying to do a simple function using iterators.

you have to enter values in the command line and it should print in the screen the smallest one.

the code is compiling but in the execution always appears zero as the answer.



#include <iostream>
#include <vector>
#include <string>
using namespace std;

int min (vector <int> valores, int n);

int main(int argc, char* argv[])
{

int i,m;
vector <int> valores;

for(i=0; i< (argc-1); i++)
valores.push_back(atoi(argv[i+1]));


m= min (valores, argc-1);

cout << m<< endl<<endl;


return 0;
}


int min (vector <int> valores, int n)
{
int i;
vector<int> ::iterator it = valores.begin();

for (it= valores.begin(); it != valores.end(); it++)
{
if (*it < *(it+1))
it = ( it + 1 );
}
return *it;
}










what is wrong with my function ??

[jsmith]

You are accessing one element beyond the end of the vector. (Think about *(it+1) when
it points to the last element in the vector)

[Chewbob]

int min (vector <int> valores, int n)
{
    int i = 0;

    for (vector<int>::iterator it = valores.begin(); it != valores.end(); it++)
    {
        if (*it > i) i = *it;
    }
    return i;
}

[R0mai]

Chewbob : what if all elements of valores are over 0?

[firedraco]

Yeah, don't assign 0 to i. Assign the first element of valores (or any element really) to i.

[jsmith]

If this isn't homework, then use std::min_element().

std::cout << *std::min_element( valores.begin(), valores.end() );

If this is homework, you should not pass vectors by value to functions; this results in a copy of the vector
which can be expensive for large vectors. Also, n is unused in your function.

[EDIT: above code assumes valores is not an empty vector]

[Chewbob]

@R0mai and firedraco

Didn't think of that. Also got the wrong operator on line 7. Should be "<".