Array not being assigned correctly

Question

I'm trying to write a program that accepts user input for a file name. From there it stores the numbers in the file into an array, sorts them, then displays them. However, i'm getting large numbers similar to accessing an out-of-bounds array, but I can tell from the debugger i'm not.

  #include <iostream>
using namespace std; 

class TestScores
{ 
public:
    TestScores();

    TestScores(int scores);

    ~TestScores();

    void AddScore(int newScore);

    void DisplayArray();

    void SortScores();

    bool ArraySorted();

    int AvgScore();

private:
    int *scoresArray;  //Dynamically allocated array 
    int numScores;  //number of scores input by user
    int scoreCounter;
    const static int default_NumArrays=10; //Default number of arrays
};

#include <iostream>
#include "TestScores.h"

TestScores::TestScores()
{
    scoresArray=new int[default_NumArrays];
    scoreCounter=0;
    numScores=default_NumArrays;
}

TestScores::TestScores(int scores)
{
    scoresArray=new int[scores];
    numScores=scores;
    scoreCounter=0;
    for(int i=0; i<scores;i++)
        scoresArray[i]=0;
}

TestScores::~TestScores()
{
    delete[] scoresArray;
}

void TestScores::AddScore(int newScore)
{
    if(scoreCounter<numScores){
        scoresArray[scoreCounter]=newScore;
        scoreCounter++;
    }
    else
        cout<<"More scores input than number of scores designated"<<endl;
}

void TestScores::DisplayArray()
{
    for(int i=0; i<numScores; i++)
        cout<<scoresArray[i]<<endl;
    cout<<endl<<"This is scoresArray"<<endl;
}

bool TestScores::ArraySorted()
{
    for(int i=0; i<(scoreCounter-1);i++){
        if(scoresArray[i]<=scoresArray[i+1])
            continue;
        else
            return false;
    }
    return true;
}

void TestScores::SortScores()
{
    int tempValue;

    while(ArraySorted()!=true){
        for(int i=0; i<(scoreCounter-1); i++){
            if(scoresArray[i]<=scoresArray[i+1])
                continue;
            else{
                tempValue=scoresArray[i+1];
                scoresArray[i+1]=scoresArray[i];
                scoresArray[i]=tempValue;
            }
        }
    }
}


int TestScores::AvgScore()
{
    int sumScores=0;

    if(scoreCounter>0){
        for(int i=0; i<scoreCounter; i++)
            sumScores+=scoresArray[i];
        return (sumScores/scoreCounter);
    }
    else{
        cout<<"There are no scores stored."<<endl;
        return 0;
    }
}

#include "TestScores.h"
#include <iostream>
#include <fstream>
#include <string>

using namespace std; 
//Function prototypes
bool FileTest(ifstream& inData);
void StoreScores(ifstream& inData, int& newNumScores, TestScores satScores);

int main()
{   
    int newNumScores=0;
    string inputFile;   //Contains name of the user file being used

    //Opening file stream
    ifstream inData;

    //User prompt for input file
    cout<<"Please enter the file name containing the student scores you wish to "
        <<"have stored, sorted, and displayed."<<endl;

    cin>>inputFile;

    //Opening file streams
    inData.open(inputFile.c_str());

    while(FileTest(inData)==false){
        cout<<"I'm sorry, the file you entered was not a valid file.  "
            <<"Please enter another file name, or enter q to exit"<<endl;
        cin>>inputFile;
        if(inputFile=="q")
            return 0;

        //Opening file streams
        inData.open(inputFile.c_str());
    }

    inData>>newNumScores;  
    TestScores satScores(newNumScores);   //Instantiating TestScores variable
    StoreScores(inData, newNumScores, satScores);  //Storing scores into array

    satScores.DisplayArray();
    satScores.SortScores();
    satScores.DisplayArray();
    cout<<endl<<"This is the array after sorting"<<endl<<endl;

    cout<<"This is the average score "<<satScores.AvgScore()<<endl;

    //Program pauses for user input to continue
    char exit_char; 
    cout<<"\nPress any key and <enter> to exit\n";
    cin>>exit_char;

    inData.close();

    return 0;   
}


bool FileTest(ifstream& inData)
{
    if(!inData)
    {
        cout<<"Your file did not open.\n";
        return false;
    }
    return true;
}


void StoreScores(ifstream& inData, int& newNumScores, TestScores satScores)
{   
    int userScore;
    while(inData>>userScore){
        satScores.AddScore(userScore);
    }
}

My test file is random.dat and contains the following:

Based on looking through the debugger, I can tell that scoreCounter is incrementing correctly and that newScore contains the next value, so why isn't it being stored in the array? Thanks for the help

What exactly are those large numbers? It could be something related to the problem of [this question and my answer](http://stackoverflow.com/questions/5447002/how-to-read-a-birthday-with-symbols-and-from-a-txt-file/5447085#5447085) of the text file being encoded in Unicode, which C++ doesn't recognize by itself. — Xeo, Mar 29 '11 at 01:26
it Looks like this 15 10-digit # 1 0 0 neg. 8-digit # neg 10-digit # and a couple more negative 10-digit numbers — ChadM, Mar 29 '11 at 01:30
@Chad: Okay, I did a quick test with your code and I get the same numbers, and an assertion failure after exiting the app. Please stay steady while I research the error. :) — Xeo, Mar 29 '11 at 01:37
Your code has broken the [rule of three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). — GManNickG, Mar 29 '11 at 01:45
@GMan: You beat me by a few seconds. Is it acceptable to close this type of question as a dupe of the Rule-of-Three FAQ? — Ben Voigt, Mar 29 '11 at 01:49
@Ben: ... no? There is another problem aside from that, on why the output isn't correct. — Xeo, Mar 29 '11 at 01:50
@Xeo: Because the array got shared between the object in main and the object in `StoreScores`, and then the local variable in `StoreScores` deleted the array, overwriting the data owned by `main`. The rule-of-three violation *causes* the data corruption. (Then there's a lurking issue about which copy the scores get added to, but the computer doing exactly what you asked it to is a feature, not a bug.) — Ben Voigt, Mar 29 '11 at 01:56
@Ben: I'd say yes, but I've gotten conflict from it in the past. (My take was that if information found in another question solves the current problem, then the question at hand is a duplicate of said question. But there was dissent. Haven't looked to see what the 'official' stance was.) — GManNickG, Mar 29 '11 at 02:18

score 2 · Accepted Answer · answered Mar 29 '11 at 01:44

2

Okay, the problem is pretty simple: You pass satScores by value to StoreScores. This will only fill the local copy, change the signature of StoreScores to the following to fix:

void StoreScores(ifstream& inData, int& newNumScores, TestScores& satScores)

(Btw you don't actually use the newNumScores variable.) The output is then as expected:

For further improvement of your code, see GMan's comment and Ben's answer.

answered Mar 29 '11 at 01:44

Xeo

129,499
52
291
397

Yeah that works...btw, how am I not using newNumScores? I'm using it to call the parameterized constructor I thought with newNumScores defining the number of arrays, which is confirmed by the fact that there are 15 arrays. I may still be wrong though – ChadM Mar 29 '11 at 01:51
@Chad: I meant in the `StoreScores` function. :) – Xeo Mar 29 '11 at 01:51

score 2 · Answer 2 · edited May 23 '17 at 09:58

2

You have a user-defined destructor but no copy-constructor or assignment operator. No wonder assignment doesn't work right, you get a leak of one buffer and double-free of the other.

Follow the Rule of Three. Or better yet, use a container that's already been debugged, like std::vector.

edited May 23 '17 at 09:58

Community

1
1

answered Mar 29 '11 at 01:46

Ben Voigt

277,958
43
419
720

Array not being assigned correctly

2 Answers2