C++ Program Functions

Question

I am having trouble with my functions. When I use a function to manipulate an array, and print it and move on to the next manipulation function, it uses the array that was previously manipulated instead of the original array. For example, when my function converts every negative number to a positive, I call the next function which zeros out all even numbers, and my array prints out all zeros, instead of using the array from the original.

#include <iostream>
#include <fstream>
#include <cstdlib>

using namespace std;

#define NUMS_PER_LINE 10    // maximum numbers to be printed on each line.

int numbers[100];   // array to hold upto 100 integer numbers.
int numcnt;     // actual count (<=100) of numbers in above array.

// reads file content into array

void read_array_from_file (const char filename[])
{
    ifstream inpfile(filename);

    if (!inpfile.is_open())
    {
        cout << "Can't open file : " << filename << endl;
        exit(1);
    }

    numcnt=0;   // Initialise count of read numbers

    // Read numbers from the file into array.
    inpfile >> numbers[numcnt];

    while (!inpfile.eof())      // Read until EOF is reached.
    {
        numcnt++;   // Got one more number from input file.
        inpfile >> numbers[numcnt];
    }

    inpfile.close();

    return;
}

// Print out all the values in the array

void print_array_content (int numsinaline)
{
    int i;

    for (i=0; i<numcnt+1; i++)
    {
        if ((i % numsinaline) == 0)
            cout << endl;
        cout << numbers[i] << " ";
    }

    cout << endl << endl;

    return;
}

// calculate average

double calculate_average ()
{
    int i;
    float sum=0;

    for (i=0; i<(numcnt-1); i++)
    {
       sum += numbers[i];
    }

    return (sum/(numcnt-1));
}

// Find numbers larger and smaller than the average.

void find_numbers_smaller_and_larger_than_average (int &larger, int &smaller, int  average)
{
    int i;

    for (i=0; i<(numcnt-1); i++)
    {
        if (numbers[i] < average)
            smaller++;
        else if (numbers[i] > average)
            larger++;
    }

    return;
}

// Convert negative numbers to positive in the array 'numbers'.

void convert_negative_to_positive ()
{
    int i;

    for (i=0; i<(numcnt-1); i++)
    {
        if (numbers[i] < 0)
            numbers[i] *= -1;
    }

    return;
}



// Convert all even numbers into zero.
void zero ()
{
    int i;

    for (i=0; i<numcnt; i++)
    {
        if (numbers[i] > 0)
            numbers[i] *= 0;
    }

    return;
}

Can you clarify "instead of using the array from the original"? — larrylampco
– larrylampco, Commented Sep 10, 2013 at 19:43
you shouldn't have return; when the method is suppose to return void. — progrenhard
– progrenhard, Commented Sep 10, 2013 at 19:44
@progenhard: That's not really the problem here (and other than that it's perfectly valid). — Zeta
– Zeta, Commented Sep 10, 2013 at 19:45
looks like you are using ONE global array."numbers[i] *= -1;" modifies this original array. you need to make a copy first if you want to continue with the original one — bitluni
– bitluni, Commented Sep 10, 2013 at 19:45
for loops shoudl start at 0 to numcnt (for (i=0; i<=numcnt; i++)), some of your functions are not working properly — Roozbeh Zabihollahi
– Roozbeh Zabihollahi, Commented Sep 10, 2013 at 19:46

Zac Howland · Accepted Answer · 2013-09-10 19:51:51Z

First of all, you are using a global variable for your array, so you are never passing it to your function. When you change a global variable in the function, it changes the data in the array. You should be passing that data into the function and NOT using global variables.

Second, while(!inpFile.eof()) is bad! Don't do it.

For file streams:

std::vector<int> numbers;
std::ifstream fin("myfile");
std::copy(std::istream_iterator<int>(fin), std::istream_iterator(),  std::back_inserter<vector<int> >(numbers));

Those 3 lines will read the entire file into the vector "numbers".

Third, when declaring your functions, pass the array:

void myFunction(const std::vector<int>& vec); // if you aren't going to change the vector

or void myFunction(std::vector& vec); // if you are going to change it

and you would call it by simply:

myFunction(numbers);

P0W · Accepted Answer · 2013-09-10 19:47:41Z

0

" it uses the array that was previously manipulated instead of the original array."

Obviously because you have your array declared globally

int numbers[100];

Outside all functions.

When you perform one operation on this array, the element get modified and the new values will be used for next functions.

Instead of this, save of copy of your original array and then use this copy whenever you wish to work on original array

answered Sep 10, 2013 at 19:47

P0W

48.1k9 gold badges78 silver badges124 bronze badges

2 Comments

Allexey Over a year ago

when you say use a copy do you mean I declare the array outside the functions and then copy the array into each function

P0W Over a year ago

@Allexey nah not like that. I meant, like you can have another array say int backup_numbers[100];, after initially filling numbers copy its content into backup_numbers using memcpy or just a simple for loop, and then whenever you need to work on original array, restore back numbers from backup_numbers using a for loop or memcpy

Zeta · Accepted Answer · 2013-09-10 19:48:15Z

All your operations act on a single global variable, numbers. If you modify it in any of your functions its values will also change in every other occurrence.

Instead, provide a way to tell your functions which array you want to use, how many elements it contains and use several arrays. This also enables you to get rid of the global variable.

Example:

#include <iostream>

using namespace std;

typedef unsigned int array_size_t;

void print_array(int array[], array_size_t size){
  for(array_size_t i = 0; i < size; ++i){
    cout << array[i] << endl;
  }
}

int main(){
  int a1[] = {1,2,3,4};
  int a2[] = {1,3,3,7,0,0,0,0};
  print_array(a1,4);
  print_array(a2,8);  
}

Remark

If you're allowed use standard containers such as std::vector instead. The solution above is more C than C++-like.

BigTailWolf · Accepted Answer · 2013-09-10 20:00:28Z

0

You are using global variable. All your operation on numbers whatever index will change your certain position's value.

Another potential risk is if your input file contains more than 100 integers, you will do

inpfile >> numbers[100];

or some index number greater than 100. which will cause a segmentation fault.

You should be very careful when using global variables

answered Sep 10, 2013 at 20:00

BigTailWolf

1,0288 silver badges18 bronze badges

1 Comment

Allexey Over a year ago

thank you, i did not understand the difference between global arrays until now!

Tricky12 · Accepted Answer · 2013-09-10 20:07:03Z

0

You are directly manipulating your array inside of your functions since it is defined globally, instead of passing in a copy as a parameter.

void modify(int[] array) {

    //Modify copied array here

}

int main() {
    int numbers[100];
    int copyNumbers[100];
    //Copy numbers
    memcpy(copyNumbers, numbers, sizeof(numbers));

    modify(copyNumbers);
    //Use modified array
    memcpy(copyNumbers, numbers, sizeof(numbers)); //Set back to original
    modify(copyNumbers);  //Modify copy again as original
}

edited Sep 10, 2013 at 20:07

answered Sep 10, 2013 at 19:52

Tricky12

6,8301 gold badge30 silver badges36 bronze badges

2 Comments

BigTailWolf Over a year ago

Your copyNumbers = numbers is an invalid assignment which will cause a compiling error.

Tricky12 Over a year ago

You are correct, thanks. Been a while since I used C++. I fixed it.

Collectives™ on Stack Overflow

C++ Program Functions

5 Answers 5

Comments

2 Comments

Example:

Remark

Comments

1 Comment

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

2 Comments

Example:

Remark

Comments

1 Comment

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related