Simple program with 2 functions doesn`t work

Question

I'm new to C++ and trying to make two simple functions, but something goes wrong.

I am trying to do the following:

1.Function for input some data.
2.Function to show what data is input.

I just want to make it simple. The code I wrote so far is:

#include <iostream>
void masiv()
{
  int x[10];
  int n, i;
  int min;
  int max=0, imax=0, imin;

  cout << "Enter the number of elements: ";
  cin >> n;

  for(i=0; i < n; i++)
  {
      cout << "Input value for x["<<i<<"]=";
      cin >> x[i];

  if (min > x[i])
  {
      min = x [i];
      imin = i;
  }

  if (max < x[i])
  {
     max = x[i];
     imax = i;
  }
}
void rezult()
{
  cout << "the smallest value on is xthe biggest value on is x["<<imin<<"]=" << min <<endl;
  cout << "nai golqmata stoinost e na x["<<imax<<"]=" << max <<endl;
}
void main()
{
  masiv();
  rezult();
}

I got bunch of errors. I know this is poor code but as I mentioned I'm just starting. Thanks

P.s. Sorry for my English

Edit: Working with this code.

#include <iostream>
using namespace std;

void masiv(int& min, int&max)
{
 int x[10];
 int n;
 int i;
 int imin, imax;
 cout << "Enter the number of elements: ";
 cin >> n;
 for(i=0; i < n; i++)
 {
  cout << "Input value for x["<<i<<"]=";
  cin >> x[i];
  if(min > x[i])
  {
    min = x [i];
    imin = i;
  }
  if(max < x[i])
  {
    max = x[i];
    imax = i;
  }
 }
}

 void rezult(int min, int max)
{
 cout << "the smallest value on is x= " << min << endl;
 cout << "the biggest value on is x= " << max << endl;
 system ("pause");
}

int main(int argc, char** argv)
{
 int min = 999999; 
 int max = -999999;
 masiv(min,max);
 rezult(min,max);
 return 0;
}

1. You didn't initialize `min`. 2. `rezult` won't know about the variables you create inside another function unless you pass them in. 3. Don't use `void main`. Use `int main`. 4. What if they enter a number greater than 10 for the number of elements? A vector fits nicely there. — chris, Jun 15 '12 at 06:24
Before manipulating variables, you need to study about the scope of variables. — Ebad Masood, Jun 15 '12 at 06:25

score 6 · Accepted Answer · answered Jun 15 '12 at 06:35

6

The min variable is never initialized, it should be initialized to a large value.

You declare an array int x[10]; but later you let user enter the number of values cin>>n without checking if it is larger than 10 or negative. This could cause an issue.

The max and min are declared only in the function masiv() they cannot be reached outside the function. If you want to make them accessible you could for instance pass them to the function instead of declaring them inside the function:

void masiv(int& min, int&max) // pass by reference
{...}

void rezult(int min, int max)
{...}

int main(int argc, char** argv) // proper main prototype
{
   int min = 999999; 
   int max = -999999;
   masiv(min,max);
   rezult(min,max);
   return 0;
}

edit : and add using namespace std; at start of file

#include <iostream>
using namespace std;

answered Jun 15 '12 at 06:35

AndersK

35,813
6
60
86

2

I'd go with `INT_MAX` or `std::numeric_limits` over 999999. It's a good habit to get into. – chris Jun 15 '12 at 06:37
1

@chris `std::numeric_limits` - the asker may not know enough about templates to infer the addition of `` – moshbear Jun 15 '12 at 06:43
1

Yes, i don't know about templates yet. I thought that the problem is something with global and local variables. @Anders now everything make sens. Thank you for your respond and for everyone else. I will accept your answer and will try harder to make this. – Jason Paddle Jun 15 '12 at 06:46
@moshbear, It was more a comment for the answer, but I see your point. At OP, all it is is a clear way to get the maximum or minimum value a type can hold, which you want to start off keeping track of the max or min of a set. It's the same logic as in the answer, but a non-hardcoded, at the very end of the scale approach. – chris Jun 15 '12 at 06:47
@Anders ok now is working! Thank you i will edit first post with working code – Jason Paddle Jun 15 '12 at 07:07
Just one more thing. With this code working but if i try to input more than 14 values the program crash. Any idea why ? Because of memory ? – Jason Paddle Jun 15 '12 at 08:00

juanchopanza · Answer 2 · 2012-06-15T07:28:10.910

At the very least, you have to qualify the namespace for cout, cin and endl, all of which are in namespace std.

As for the problems with the locally-scoped variables, I would change the signature of masiv to return an std::pair<int,int> containing the min and max values:

typedef std::pair<int,int> MinMax;

MinMax massiv() { .... }

Why nor pass min and max by reference? because you depend on the values of the references passed. You would have to check whether they are reasonable and so on. Returning the minimum and maximum from the function itself puts the function in full control.

score 2 · Answer 3 · answered Jun 15 '12 at 06:38

2

You need to think about data flow here.

Your main function executes two functions, but how does the data get out of the massiv function or into the result function?

You can use globals, or you can have your main structured more like:

void main()
{
    int x[10];
    massiv(x);
    rezult(x);
}

The rezult function should be processing the results in x and to populate the min and max variables. Move the if statements from massiv to rezult.

answered Jun 15 '12 at 06:38

Daniel

51
2

'rezult' really? Sure the yanks don't mess up that word as well, it's not even said with a 'z' sound – thecoshman Jun 15 '12 at 06:43
it's knowingly 'rezult' instead of 'result' – Jason Paddle Jun 15 '12 at 06:52

score -3 · Answer 4 · answered Jun 15 '12 at 06:28

-3

first of all, all your variables are locally defined in massiv() function, make the global first.

answered Jun 15 '12 at 06:28

dpanshu

453
3
14

2

Please, making them global is not the solution. To work with the code there, make the first function take `min`, `max`, `imin`, and `imax` by reference, and the second function take them by value. Then create those 4 in `main`. The real problem, however, is that the function's doing more than one function's work. It should be split. – chris Jun 15 '12 at 06:29
@chris: The guy who asked this question is a newbee and will understand concepts gradually. Take it easy. – Ebad Masood Jun 15 '12 at 06:38
@ebad86, I'm just trying to steer them away early. Everything would be a lot easier to handle if the function was cut down into smaller ones that each did their part. Passing 4 variables in just for that should, and does seem excessive. – chris Jun 15 '12 at 06:43
@user1439164, see this: http://stackoverflow.com/questions/484635/are-global-variables-bad – chris Jun 15 '12 at 06:44

score -3 · Answer 5 · answered Jun 15 '12 at 06:30

-3

you imin, min, imax and max should make global for function rezult() to access them.

answered Jun 15 '12 at 06:30

yongzhy

979
8
18

There's no reason for those to be global. – GManNickG Jun 15 '12 at 06:35

Simple program with 2 functions doesn`t work

5 Answers5