SegFault Problem

[KennethQ]

In my computer science class, we were to take the following .txt file and arrange it so that it takes in the first two names in two separate strings and the numbers in an array with 12 elements.

John Smith 90 84 77 43 62 93 100 87 69 87 77 90
Judy Apple 65 47 93 78 82 49 78 79 59 63 59 92
Bonnie Jones 92 45 72 68 49 47 83 81 76 69 90 85
Ralph Peters 78 45 29 48 37 42 55 69 56 34 72 44
Alice Rivers 67 73 87 39 57 74 89 49 58 100 0 42
Arlo Gibson 97 93 89 96 100 94 85 100 92 94 97 91
Susan Abernathy 85 69 72 39 0 0 45 89 100 59 94 90
Bob Junior 74 84 77 43 82 93 97 87 68 87 77 90
June Southland 83 47 83 78 82 49 92 79 59 64 59 92
Roger Dodger 48 49 73 68 49 42 83 81 91 69 90 85
Party Pooper 38 45 29 48 47 42 53 69 56 34 91 44
April Smith 67 53 87 52 57 74 89 49 58 100 39 42
Dianne Gomez 92 93 89 96 84 94 85 100 92 94 87 91
Debra Younger 83 71 72 39 45 92 45 89 100 59 63 90

After that, we were supposed to print a report that shows for each student the student's name, their average for the 12 grades, their high grade, their low grade, their median grade and finally a class average grade (going vertically down each grade column). The code below compiles and runs with only one error(as far as I know), which is the segfault at the end. I have no idea why it's doing this since I'm still new to c++. Can anyone help me?
My code so far:

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

void sortArray(int array[], int size);
int findGrades(int iArray[], int & iMax, int & iMin, int & iSum, double & iAverage, double & iMedian);

const int iSize = 12;

int main () 
{
	//delcare variables.
  string sFirst, sLast, filename;
  int iSum = 0;
  double iAverage = 0.0;
  int grades[iSize];
  int iMax = 0;
  int iMin = 0;
  double iMedian = 0.0;
  
  ifstream datafile;
  datafile.open( "Lab13Data.txt" );
  if (datafile.is_open())
  {
		cout << "Names" << "			" << "Grades";
	  while (!datafile.eof())
	  {
		  datafile >> sFirst;
		  
		  cout << endl << endl << sFirst << " ";
		  datafile >> sLast;
		  cout << sLast << "		";
		 for (int i = 1 ;i <= iSize ;i++) //done to get the show the array next to the names.
			{			
		   	datafile >> grades[i];
		   	if (i % 13 == 0) //separates the next set of students and their respective grades.
			    {
			   	cout << endl;
		   	} 
			    cout << grades[i] << " "; 
			    	  
			}
  		}
  		sortArray(grades, iSize);
  		findGrades(grades, iMax, iMin, iSum, iAverage, iMedian);	
  		//print out results from formulas
		cout << iMax << " is the largest value present.\n";
		cout << iMin << " is the smallest value present.\n";
		cout << iMedian << " is the median of all of the numbers.\n";
		cout << iAverage << " is the average of all of the numbers.\n";
	}
	else
		cout << "Unable to open file\n"; //returns an error message if the file could not open.
	datafile.close();//closes file
	

}

//***********************************************************
// Definition of function sortArray                         *
// This function performs an ascending order bubble sort on *
// array. size is the number of elements in the array.     *
//***********************************************************

void sortArray(int array[], int size)
{
   bool swap;
   int temp;

   do
   {
      swap = false;
      for (int count = 0; count < (size - 1); count++)
      {
         if (array[count] > array[count + 1])
         {
            temp = array[count];
            array[count] = array[count + 1];
            array[count + 1] = temp;
            swap = true;

         }   
      }
   } while (swap);

}

int findGrades(int iArray[], int & iMax, int & iMin, int & iSum, double & iAverage, double & iMedian)
{
	int index = 0;
	
	for(iArray[index] >= iMax; index++;) //for loop used to find the maximum value
		{
			iMax = iArray[index];
		}
	for(iArray[index] <= iMin; index++;) //for loop used to find the minimum value
		{
			iMin = iArray[index];
		}
	for(iArray[index] <= iSum || iArray[index] >= iSum; index++;) //for loop used to sum up the int values
		{
			iSum += iArray[index];
		}
			iAverage = static_cast<double>(iSum)/iSize; //changed iSum to a double value so nothing would get truncated.
}

What I get:

Names                   Grades

John Smith              90 84 77 43 62 93 100 87 69 87 77 90 

Judy Apple              65 47 93 78 82 49 78 79 59 63 59 92 

Bonnie Jones            92 45 72 68 49 47 83 81 76 69 90 85 

Ralph Peters            78 45 29 48 37 42 55 69 56 34 72 44 

Alice Rivers            67 73 87 39 57 74 89 49 58 100 0 42 

Arlo Gibson             97 93 89 96 100 94 85 100 92 94 97 91 

Susan Abernathy         85 69 72 39 0 0 45 89 100 59 94 90 

Bob Junior              74 84 77 43 82 93 97 87 68 87 77 90 

June Southland          83 47 83 78 82 49 92 79 59 64 59 92 

Roger Dodger            48 49 73 68 49 42 83 81 91 69 90 85 

Party Pooper            38 45 29 48 47 42 53 69 56 34 91 44 

April Smith             67 53 87 52 57 74 89 49 58 100 39 42 

Dianne Gomez            92 93 89 96 84 94 85 100 92 94 87 91 

Debra Younger           83 71 72 39 45 92 45 89 100 59 63 90 

Segmentation fault

[Catfish3]

A segmentation fault usually happens when you try to use memory that you shouldn't.

In your case the culprit is...
for (int i = 1 ;i <= iSize ;i++)

... because in C++ counting starts from 0, not 1.
So the first element in the grades array is grades[0], not grades[1].
Last element is grades[iSize - 1], not grades[iSize].

Corrected for() loop:
for (int i = 0 ;i < iSize ;i++)

[KennethQ]

I realized that and tried it too but the results turned out to be worse than before, unfortunately :/. However when i change it to what you put as the corrected for loop, it outputted:

Names                   Grades

John Smith
90 84 77 43 62 93 100 87 69 87 77 90 

Judy Apple
65 47 93 78 82 49 78 79 59 63 59 92 

Bonnie Jones
92 45 72 68 49 47 83 81 76 69 90 85 

Ralph Peters
78 45 29 48 37 42 55 69 56 34 72 44 

Alice Rivers
67 73 87 39 57 74 89 49 58 100 0 42 

Arlo Gibson
97 93 89 96 100 94 85 100 92 94 97 91 

Susan Abernathy
85 69 72 39 0 0 45 89 100 59 94 90 

Bob Junior
74 84 77 43 82 93 97 87 68 87 77 90 

June Southland
83 47 83 78 82 49 92 79 59 64 59 92 

Roger Dodger
48 49 73 68 49 42 83 81 91 69 90 85 

Party Pooper
38 45 29 48 47 42 53 69 56 34 91 44 

April Smith
67 53 87 52 57 74 89 49 58 100 39 42 

Dianne Gomez
92 93 89 96 84 94 85 100 92 94 87 91 

Debra Younger
83 71 72 39 45 92 45 89 100 59 63 90 

Debra Younger
Segmentation fault

Instead.

[Catfish3]

Hmm.
In your findGrades() function, you don't have a limit for index, nor do you reset it between checks. So you may get out of bounds there, too.

[KennethQ]

So i should set index into each for loop as the reset between each loop? Also i would have thought that the error occurs before it even reaches the first function call.

[Catfish3]

So i should set index into each for loop as the reset between each loop?

Well all you need to do is change the start to: for (index = 0;.
Even then, index could possibly go outside range, because you don't check it against a limit (which is the array's size).

Also i would have thought that the error occurs before it even reaches the first function call.

You could add cerr << "I'm in function x()" << endl; in the functions to be sure.

[KennethQ]

Ok, i'll try that. Also I think instead of having to even deal with the unlimited index, I could just change every for statement into f statements like so:

if(iArray[index] <= Array[index + 1])//if loop used to find the minimum value
		{
			iMin = iArray[index];
                        index++;
		}

And then put a do/while that says to do that while index < 12. Could that possibly work?

[Catfish3]

That doesn't work because you're not checking the minimum value against iMin.
So if you have an iArray of {3, 4, 5, 6, 100, 200}, at the end iMin would be 100 instead of 3.

And then, what's the purpose of doing this, if you sort the grades array? If it's sorted by sortArray(), just grab its first and last elements for iMin and iMax respectively.

I'd like to go a bit off topic.

How free are you, to use the C++ language at its fullest?
C++ already has your functions in its library, which can sort for you, swap two values for you, and find the minimum and maximum values in an array for you.

http://cplusplus.com/reference/algorithm/sort/
http://cplusplus.com/reference/algorithm/swap/
http://cplusplus.com/reference/algorithm/min_element/
http://cplusplus.com/reference/algorithm/max_element/

If you're totally new to C++, they may be tricky to use at first.
But your program will end up being cleaner, and simpler.

[KennethQ]

I can use any means necessary to get the job done. I also forgot to change my old program function to this one:

int findGrades(int iArray[], int & iMax, int & iMin, int & iSum, double & iAverage, double & iMedian)
{
	cerr << "I'm in function x()" << endl;
	int temp = 0;
	int index = 0;
	do
	{
					if(iArray[index] >= iArray[index + 1] )
					{
						temp = iArray[index];
						if(temp >= iMax)
						{
							iMax = temp;
							index++;
						}
					}
					if(iArray[index] <= iArray[index + 1])
					{
						temp = iArray[index];
						if(temp <= iMin)
						{
							iMin = temp;
							index++;
						}
					}
					if(iArray[index] <= iArray[index+1] || iArray[index] >= iArray[index+1])
					{
						 temp = iArray[index];
						 iSum = iSum + temp;
						 index++;
					}
					iAverage = static_cast<double>(iSum)/12; //changed iSum to a double value so nothing would get truncated.
  }while(index < 12);
}

Showing what I was actually going to do. Actually i hadn't even thought of taking those values from the sorted array, until you mentioned it i'll change that too.

[Catfish3]

Here's how I would do it, with some minor differences.
This code isn't meant to intimidate, but to inspire. Whatever questions you have, I'll be around to answer them.

From what I tested, it runs on your input data, and I didn't see obvious bugs. (If anyone does, please point them out.)

#include <algorithm>
#include <fstream>
#include <iomanip>
#include <iostream>
#include <istream>
#include <numeric>
#include <list>
#include <ostream>
#include <sstream>
#include <string>

struct Student {
    std::string firstName;
    std::string lastName;
    std::list<int> gradesList;

    int highestGrade() const
    {
        return *std::max_element(gradesList.begin(), gradesList.end());
    }

    int lowestGrade() const
    {
        return *std::min_element(gradesList.begin(), gradesList.end());
    }

    float medianGrade() const
    {
        return static_cast<float> (std::accumulate(gradesList.begin(), gradesList.end(), 0)) / gradesList.size();
    }
};

// this function "teaches" an input stream (which can be a file) to read a Student
std::istream & operator >> (std::istream &is, Student &s)
{
    std::string line;

    // read a whole line, with name and grades, and ignore empty lines
    while (is.good() && line.length() == 0)
        std::getline(is, line);

    if (!line.empty()) // only proceed if actually read anything
    {
        // put the line in a string stream to easily extract elements
        std::istringstream lineStream(line);

        lineStream >> s.firstName;
        lineStream >> s.lastName;
        s.gradesList.clear(); // clear old grades

        int tempGrade;

        while (lineStream >> tempGrade) // if we read a grade,
            s.gradesList.push_back(tempGrade); // add it to the list
    }

    return is;
}

// this function template "teaches" an output stream to display an std::list
template <typename Type>
std::ostream & operator << (std::ostream &os, const std::list<Type> &l)
{
    for (typename std::list<Type>::const_iterator ci = l.begin(); ci != l.end(); ++ci)
        os << *ci << ' ';

    return os;
}

// this function "teaches" an output stream (which can be std::cout) to display a Student
std::ostream & operator << (std::ostream &os, const Student &s)
{
    os << "\nStudent name: " << s.firstName << ' ' << s.lastName << '\n';
    os << "Grades: " << s.gradesList << '\n';
    os << "Highest grade: " << s.highestGrade() << '\n';
    os << "Lowest grade: " << s.lowestGrade() << '\n';
    os << "Median grade: " << s.medianGrade() << '\n';
    os << std::setfill('-') << std::setw(20) << '\n';
    return os;
}

int main()
{
    std::ifstream inputFile("KennethQ.input.txt");
    std::list<Student> studentsList;
    Student tempStudent;

    while (inputFile >> tempStudent) // if we read a student,
        studentsList.push_back(tempStudent); // add him/her to the list

    std::cout << "Report:\n\n";
    std::cout << studentsList << "\n\n";

    float tempGrade = 0.0f;

    for (std::list<Student>::const_iterator ci = studentsList.begin(); ci != studentsList.end(); ++ci)
        tempGrade += (*ci).medianGrade();

    tempGrade /= studentsList.size();

    std::cout << "Class average grade is: " << tempGrade << std::endl;
}

[Chervil]

Function float medianGrade() is returning the average (or arithmetic mean), not the median.

[Catfish3]

@ Chervil: thanks, could you point me to the formula for that?

[Chervil]

@ Catfish3 To be frank, it's years since I had to use this, so I'm dependent on what Google has come up with: http://www.mathsisfun.com/median.html

[KennethQ]

I see... Well, thank you for the program, I'll add some parts of that program to mine and take out what I feel could be unnecessary or not as well made. After I understand it all of course. If I have any questions then I'll bring them up here. Thanks again!!

[Catfish3]

Thanks Chervil.

Here's the updated version. Posted in full because some things changed outside the Student struct as well.

#include <algorithm>
#include <fstream>
#include <iomanip>
#include <iostream>
#include <istream>
#include <iterator>
#include <numeric>
#include <list>
#include <ostream>
#include <sstream>
#include <string>

struct Student {
    std::string firstName;
    std::string lastName;
    std::list<int> gradesList;

    int highestGrade() const
    {
        return *std::max_element(gradesList.begin(), gradesList.end());
    }

    int lowestGrade() const
    {
        return *std::min_element(gradesList.begin(), gradesList.end());
    }

    float averageGrade() const
    {
        return static_cast<float> (std::accumulate(gradesList.begin(), gradesList.end(), 0)) / gradesList.size();
    }

    float medianGrade() const
    {
        float result;
        std::list<int>::const_iterator ci = gradesList.begin();

        if (gradesList.size()%2 == 0) // even number of grades
        {
            std::advance(ci, gradesList.size()/2 - 1);
            result = *ci;
            std::advance(ci, 1);
            result += *ci;
            result /= 2;
        }
        else // odd number of grades
        {
            std::advance(ci, gradesList.size()/2);
            result = *ci;
        }

        return result;
    }
};

// this function "teaches" an input stream (which can be a file) to read a Student
std::istream & operator >> (std::istream &is, Student &s)
{
    std::string line;

    // read a whole line, with name and grades, and ignore empty lines
    while (is.good() && line.length() == 0)
        std::getline(is, line);

    if (!line.empty()) // only proceed if actually read anything
    {
        // put the line in a string stream to easily extract elements
        std::istringstream lineStream(line);

        lineStream >> s.firstName;
        lineStream >> s.lastName;
        s.gradesList.clear(); // clear old grades

        int tempGrade;

        while (lineStream >> tempGrade) // if we read a grade,
            s.gradesList.push_back(tempGrade); // add it to the list

        s.gradesList.sort(); // sort the grades
    }

    return is;
}

// this function template "teaches" an output stream to display an std::list
template <typename Type>
std::ostream & operator << (std::ostream &os, const std::list<Type> &l)
{
    for (typename std::list<Type>::const_iterator ci = l.begin(); ci != l.end(); ++ci)
        os << *ci << ' ';

    return os;
}

// this function "teaches" an output stream (which can be std::cout) to display a Student
std::ostream & operator << (std::ostream &os, const Student &s)
{
    os << "\nStudent name: " << s.firstName << ' ' << s.lastName << '\n';
    os << "Grades: " << s.gradesList << '\n';
    os << "Highest grade: " << s.highestGrade() << '\n';
    os << "Lowest grade: " << s.lowestGrade() << '\n';
    os << "Average grade: " << s.averageGrade() << '\n';
    os << "Median grade: " << s.medianGrade() << '\n';
    os << std::setfill('-') << std::setw(20) << '\n';
    return os;
}

int main()
{
    std::ifstream inputFile("KennethQ.input.txt");
    std::list<Student> studentsList;
    Student tempStudent;

    while (inputFile >> tempStudent) // if we read a student,
        studentsList.push_back(tempStudent); // add him/her to the list

    std::cout << "Report:\n\n";
    std::cout << studentsList << "\n\n";

    float tempGrade = 0.0f;

    for (std::list<Student>::const_iterator ci = studentsList.begin(); ci != studentsList.end(); ++ci)
        tempGrade += (*ci).averageGrade();

    tempGrade /= studentsList.size();

    std::cout << "Class average grade is: " << tempGrade << std::endl;
}

[Catfish3]

@ KennethQ: there is a lot of room left for improvement.
For instance it doesn't even check if the file was successfully opened.

Happy bug hunting!

Edit: minor bugfix:
If the input file doesn't end with a newline, getline() will hang (for me).
I don't know why it does, but this seems to fix it.

    // read a whole line, with name and grades, and ignore empty lines
    while (line.length() == 0 && std::getline(is, line).good());