Broken code for arrays

[sr2cute702]

Hello,

Could you please help me in fixing my broken code?

I need my code to accept negative numbers and doubles. It also has to show both arrays listed next to each other.


This Code is using a linux redirection command to run the file inputs.



sample data inputs would be

0.0 13.0 27.0 -27.0 -9.0 5.0 11.0 1.0 21.0 -1.0 16.0

correct output needed.

Input Array = find evens and double them
2nd Array = find odds and find the cube root.

Input Array   2nd Array
0.00000       0.00000
13.00000      2.35133
27..00000     54.00000
-27.00000     -3.00000
-9.00000      -18.00000
5.00000       1.70000
11.00000      22.00000
1.00000       1.00000
21.00000     42.00000
-1.00000     -1.00000
16.00000     32.00000
Sum of all Values in array 1: 57.00000
Sum of first 5 values in array 2: 35.35133

my code only works for single numbers and positives.

1 2 3 4 5


my bad out put.

Input Array:
         1
2nd Array:
         4
         0
-1987082921
Sum of all values in array 1: 1.00000

Sum of first 5 values in array 2: 0.00000

  
#include <iostream>
#include <iomanip>
#include <cmath>
#include <string>
#include <fstream>

using namespace std;

const int MAX = 50;
void get_data (int[], int[], int &, int &);
double summation (int[], int);
double summation5 (int[], int);
void print_results (int[], double, double, int[], int, int);




int main ()
{

  int list01[MAX];	//array to store values from file even
  int list02[MAX];	//array to store values from file odds
  int n;			    //# values read from the file
  int N;			    //# values read from the file
  double sum01;			//sum all value in array 1
  double sum02;			//sum of 5 only value in array 2
  get_data (list01, list02, n, N);
  sum01 = summation (list01, n);
  sum02 = summation5 (list02, N);
  print_results (list02, sum02, sum01, list01, n, N);

  return 0;

}

void get_data (int list02[], int list01[], int &n, int &N)
{
  int temp;
  n = 0;
  N = 0;
  cin >> temp;
  while (!cin.eof())
    {
      
      if (temp % 2 == 0)
      {
      list01[n] = temp * 2; // evens doubled 
      n++;
      }
      else
      {
          
      if (temp % 2 != 0)
      
        list02[N] = cbrt(temp); // odds cube root
        N++;
      
      }
      cin >> temp;
    }

}

double summation (int data01[], int n)
{

  double sum1 = 0.0;
  for (int j = 0; j < n; j++)
    {
    sum1 = sum1 + data01[j];
    }

  return sum1;

}

double summation5 (int data02[], int N)
{

  double sum1 = 0.0;
  for (int j = 5; j < N; j++)
    {
    sum1 = sum1 + data02[j];
    }

  return sum1;

}

void print_results (int list02[], double sum02, double sum01, int list01[], int n, int N)
{

  cout << "Input Array:" << endl;
  for (int j = 0; j < n; j++)
    {
      cout << right << setw (10) << fixed << setprecision (5) << list01[j] <<
	endl;
    }
  cout << "2nd Array:" << endl;
  for (int k = 0; k < N; k++)
    {
      cout << right << setw (10) << fixed << setprecision (5) << list02[k] <<
	endl;
    }

  cout << fixed << setprecision (5);
  cout << "Sum of all values in array 1: " << sum01 << endl << endl;
  cout << "Sum of first 5 values in array 2: " << sum02 << endl << endl;
}

[Handy Andy]

Hello sr2cute702,

The first thing that jumps out at me is the "get_data" function. The while loop should not be checking for "eof" of any kind. It will usually process the last read twice before it will cause the while loop to fail. Second I do not think that "cin" ever sets the "eof" bit when it fails. I will see what I can come up wih when I work with the program.

Next question is this input coming from a file or "cin"?

The if and else is not quite right. It should be if/else not what you have. If it is not even than it is odd, there is no other choice or need to test for it.

if (temp % 2 == 0)
{
      list01[n] = temp * 2; // evens doubled 
      n++;
}
else
{
      list02[N] = cbrt(temp); // odds cube root
      N++;
}

Once I have a chance to run the program I will know more.

Hope that helps,

Andy

[sr2cute702]

Thanks for your help, I have set up with cin for testing but I will converted to open a file afterword's.

[keskiverto]

You example output does not match your description. You say double even values and cbtr odd values, but the example has every second value doubled and the rest rooted.

[Handy Andy]

Hello sr2cute702,

I will start at the top.

Line 10 I changed "MAX" to "MAXSIZE" because it is a better description for the variable name. Not important to change, but something to think about for the future.

In the function proto types and function definitions along with the arrays in main I changed all the "int"s to "doubles".

On line 28 you pass (list01, list02, n, N) but on line 37 you receive as (int list02[], int list01[], int &n, int &N). Notice a difference?

Line 31 Although it works and was not a problem it is better to stay consistent, i.e., (list01, Sum01, list02, Sum02, n, N). Switching the order of the "01"s and "02"s can get confusing, i.e., line 28 and the function definition.

Line 37 the "get_data" function I changed the order of the variables "01" and "02" because the input was going into the wrong arrays. I also set up the function to input from a file, Just as easy to do it now as later. Also I would have initialized the arrays in main with the same numbers to save from typing each time I tested the program. Just as easy to have the program read from a file right from the start.

The next change was to if (temp % 2 == 0). Changing "temp" to a double I had to change this line to if (static_cast<int>(temp) % 2 == 0) to work correctly. Which is OK because all you need to do is determine if what is on the lhs of the decimal point is negative or positive. In the end it still stores the floating point number read from the file. I also took out the square and cube root from the input to keep the original numbers in the arrays.

The "summation" functions work fine with the exception of "summation5". I had to change the for loop to work correctly (int j = 0; j < 5; j++). Because you only need the first five.

In the "print_results" function this is where I squared and used the "cbrt" function in the output. Call it a personal quirk of mine, but I changed the headings a bit and used "std::setw()" to line up the numbers better. Rather than trying to describe everything here is what I did:

void print_results(double list02[], double sum02, double sum01,	double list01[], int n, int N)
{

	cout << "Input Array:\t2nd Array:\n  Squared\tCube root\n" << endl;
	for (int j = 0; j < N; j++)
	{
		cout << right << fixed << setprecision(5);  // <--- Removed the setw().

		if (j < n)
			std::cout << setw(10) << list01[j] * list01[j];  // <--- Better to put the setw() here.
		
		if (j < N && j< n)
			std::cout << std::setw(14) << std::cbrt(list02[j]) << std::endl;
		else
			std::cout << setw(10) << " " << std::setw(14) << std::cbrt(list02[j]) << std::endl;
		std::cout << std::endl;
	}

	//cout << fixed << setprecision(5);  // <--- Already set.
	cout << "Sum of all values in array 1: " << sum01 << '\n' << endl;  // <--- Could also call the function here instead of passing the variable.
	cout << "Sum of first 5 values in array 2: " << sum02 << '\n' << endl;  // <--- Could also call the function here instead of passing the variable
}

One last note it is always a good idea to initialize you variables when defined. It saves headaches later.

Hope that helps,

Andy

[sr2cute702]

Andy,

did this help immensely, could you assist me with getting to read a txt file?

so its reading the first number wich sets the array size, but then i stops at 0.00

here is what the text file has

11


0.0 13.0 27.0 -27.0 -9.0 5.0 11.0 1.0 21.0 -1.0 16.0

my out put now is incorrect.

Enter the name of an input file
datafortwelve
Input Array: 2nd Array:

0.00000 0.00000
0.00000 0.00000
0.00000 0.00000
0.00000 0.00000
0.00000 0.00000
0.00000 0.00000
0.00000 0.00000
0.00000 0.00000
0.00000 0.00000
0.00000 0.00000
0.00000 0.00000
Sum of all values in array 1: 0.00000
Sum of first 5 values in array 2: 0.00000


here is my new code.

#include <iostream>
#include <iomanip>
#include <cmath>
#include <string>
#include <fstream>

using namespace std;

void get_data (double[], double[], int &, int &);
double summation (double[], int);
double summation5 (double[], int);
void print_results (double[], double, double,double[], int, int);
int main ()
{
    ifstream in;
   string infilename;
   int MAXSIZE=30;

         double list01[MAXSIZE];//array to store values from file even                                                                                                                                                 
          double list02[MAXSIZE];//array to store values from file odds                                                                                                                                                
          int n;    //# values read from the file                                                                                                                                                                      
          int N;    //# values read from the file                                                                                                                                                                      
          double sum01;//sum all value in array 1                                                                                                                                                                      
          double sum02;//sum of 5 only value in array 2    
          get_data (list01, list02, n, N);
          sum01 = summation (list01, n);
          sum02 = summation5 (list02, N);
          print_results (list02, sum02, sum01, list01, n, N);
                                                                                                                                              

  return 0;
}
void get_data (double list01[], double list02[], int &n, int &N)
{
  double temp;
  n = 0;
  N = 0;
  int MAXSIZE=30;
                                                                                                                                                     

  ifstream in;
  string infilename;
  cout << "Enter the name of an input file\n";
  cin >> infilename;
 in.open(infilename.c_str());

  if(in.is_open())
    {
      in>>MAXSIZE;

       double list01[MAXSIZE];//array to store values from file even                                                                                                                                                   
       double list02[MAXSIZE];//array to store values from file odds                                                                                                                                                   
      in>>temp;
      for (int a= 0; a<MAXSIZE; a++)
        {

          list01[n] = temp;

          list02[N] = temp;


          if (static_cast<int>(N) % 2 == 0)
            {

              list02[N] = temp * 2;                                                                                                                                                                            
            }
          else
            {

              list02[N] = cbrt(temp);                                                                                                                                                                                                 

            }
          n++;
          N++;
          in >> temp;
        }


    }
  else
    cout<<"Error: file\"" <<infilename<<"\" was not opened\n";

}
double summation (double data01[], int n)
{
  double sum01 = 0.0;
  for (int j = 0; j < n; j++)
    {
      sum01 = sum01 + data01[j];
    }
  return sum01;
}
double summation5 (double data02[], int N)
{
  double sum02 = 0.0;
  for (int j = 0; j < 5; j++)
    {
      sum02 = sum02 + data02[j];
    }
  return sum02;
}
void print_results(double list02[], double sum02, double sum01,double list01[], int n, int N)
{
  cout << "Input Array:\t2nd Array:\n" << endl;
  for (int j = 0; j < N; j++)
    {
      cout << right << fixed << setprecision(5);  // <--- Removed the setw().                                                                                                                                          
      if (j < n)
      cout << setw(11) << list01[j];  // <--- Better to put the setw() here.         

      if (j < N && j< n)
      cout <<setw(14) <<list02[j];
      else
      cout << setw(11) << " " << setw(14) << cbrt(list02[j]) <<endl;
      cout << endl;

    }                                                                                                                                                            
  cout << "Sum of all values in array 1: " << sum01 << '\n';  // <--- Could also call the function here instead of passing the variable.                                                                               
  cout << "Sum of first 5 values in array 2: " << sum02 << '\n' << endl;  // <--- Could also call the function here instead of passing the variable                                                                    
}

        

[keskiverto]

Lines 51 and 52 are illegal in standard C++. The size of local automatic array must be known during compilation. In your code the array size depends on runtime input. Standard C has VLA (variable length arrays), but C++ does not. C++ has std::vector.


On lines 51-87 you do have an array list01. It dies on line 79.
On lines 34-83 you do have an "array" list01. It cannot be seen on lines 51-87 due to local object of same name.
This is masking. Name in inner scope hides identical name of outer scope.

[sr2cute702]

I some what think I know what your saying. How would I set the array size that the file has listed then.

Then 51 and 52 would be moved or removed?

sorry im not that code savvy as of yet.

[Handy Andy]

Hello sr2cute702,

keskiverto makes a good point about lines 51 and 52, but my question is, You have sent the arrays to the function, so in the function definition the two arrays are already defined, Why do you need to redefine what you already have access to?

If you want to use "MAXSIZE" in the function take it out of main and put it above main as a global variable as constexpr int MAXSIZE{30};. This variable needs to be a "constexpr" or at least a "const" to work properly as keskiverto pointed out.

I will need a few minutes to go over your code for reading the file and do some testing/checking and I will have an answer shortly.

Hope that helps,

Andy

[sr2cute702]

thanks again for your help, the 30 was for testing purposes. in the end the first integer in the text file will set the maxsize value.

[keskiverto]

There is nothing wrong in the 30, if it is chosen wisely.

1. Create fixed size array, like you already do.
2. Read N values into the array. Do not let the N become larger than the fixed size.
3. Do something with the N values.

You don't want to make too large array, for that is unnecessary allocation, but not too small either, or you have to deal with the case that input has more data than you can handle.

This is the least "code savvy" approach.


The second approach is to manually and dynamically manage memory for the array. This is the hardest method.


Simple, dynamic and probably "best" approach is to use the std::vector.
Pros: you learn to use the vector and it will not be too small nor unnecessarily large.
Cons: you have to learn to use the vector.

[Handy Andy]

Hello sr2cute702,

This is your code with comments. I will not say that it fixes all the problems, but it does point out what does not work.

void get_data(double list01[], double list02[], int &n, int &N)
{
	double temp;
	//n = 0;  // <--- Should be initialized in main when defined. Not needed here.
	//N = 0;  // <--- Should be initialized in main when defined. Not needed here.
	//int MAXSIZE = 30;  // <--- Not needed here. Should be a global variable.

	ifstream in;

	string infilename;

	cout << "Enter the name of an input file\n";
	cin >> infilename;

	in.open(infilename.c_str());

	if (in.is_open())
	{
		std::cout << "\n File " << infilename << " is open" << std::endl;
		std::this_thread::sleep_for(std::chrono::seconds(2));  // <--- Needs header files chrono" and "thread".
	}
	else
	{
		std::cout << "\n File " << infilename << " did not open" << std::endl;
		std::this_thread::sleep_for(std::chrono::seconds(3));  // <--- Needs header files chrono" and "thread".
		exit(1);
	}

	//  This if/else is not needed with the above code.
	//if (in.is_open())
	//{
		in >> MAXSIZE;

		double list01[MAXSIZE];  //array to store values from file even.
		double list02[MAXSIZE];//array to store values from file odds.

		in >> temp;

		for (int a = 0; a < MAXSIZE; a++)  //<--- Should not be a for loop. A while loop works better
		{
			list01[n] = temp;
			list02[N] = temp;

			if (static_cast<int>(N) % 2 == 0)  // <--- "static_cast" not needed, "N" is already an int.  Also checking the wrong variable.
			{  // <--- These are not needed for one line.
				list02[N] = temp * 2;  // <--- You should only store the original number. Math can be done later.
			}  // <--- These are not needed for one line.
			else
			{  // <--- These are not needed for one line.
				list02[N] = cbrt(temp);  // <--- You should only store the original number. Math can be done later.
			}  // <--- These are not needed for one line.

			n++;
			N++;

			in >> temp;
		}


	//}
	//else
	//	cout << "Error: file\"" << infilename << "\" was not opened\n";

}

The lines 17 - 27 may be a bit much, but I found when first learning about using files it worked well. As you learn more about using files you will see how that can be shortened.

Not only is the for loop the wrong way to read a file it will not work the way you are thinking. Looping on "a < MAXSIZE" will cause the loop to execute 30 times, but if the input file only has ten numbers 2/3 of the array will be filled with the last number read.

The if statement is checking the wrong number. Whatever is in "temp" is what needs to be checked and the the "static_cast" will work.

As you will see with the next code this is a better and shorter way of reading the file:

void get_data(double list01[], double list02[], int &n, int &N)
{
	std::string iFileName{ "Input Data.txt" };

	std::ifstream inFile;

	inFile.open(iFileName);

	if (inFile.is_open())
	{
		std::cout << "\n File " << iFileName << " is open" << std::endl;
		std::this_thread::sleep_for(std::chrono::seconds(2));  // <--- Needs header files chrono" and "thread".
	}
	else
	{
		std::cout << "\n File " << iFileName << " did not open" << std::endl;
		std::this_thread::sleep_for(std::chrono::seconds(3));  // <--- Needs header files chrono" and "thread".
		exit(1);
	}

	double temp{};
	//n = 0;  // <--- Should be initialized in main and not zeroed here.
	//N = 0;  // <--- Should be initialized in main and not zeroed here.

	while (inFile >> temp)
	{
		if (static_cast<int>(temp) % 2 == 0)
		{
			list01[n] = temp; // *2; // evens doubled. Can be done later in an output. 
			n++;
		}
		else
		{
			list02[N] = temp; // cbrt(temp); // odds cube root. Can be done later in an output.
			N++;
		}
	}
	
	inFile.close();
}

Notice the difference between your code and mine. Any questions let me know.

Hope that helps,

Andy