Crossman
Crossman

Reputation: 278

Can't get string out of array when passed to constructor

I've spent the whole day trying to figure out why this doesn't want to work, I'm extracting information from a text file and sending the values through as arrays to a constructor (which works fine, I can print out the values and they will display.) But I can't create an object of another class inside of the constructor without it going into an infinite loop.

My main File:

int main(int argc, char** argv)
{
string line;
ifstream myfile ("test1.txt");
string rows [15];
int index = 0;

string tempForSize[5];
double * sizeArray = new double[5]; 
int firstIntOccur =0; 

string * arrForEquations = new string[12];
int equationCount = 0;

if (myfile.is_open())       {
    while ( myfile.good() )     {
        getline (myfile,line);
        rows[index] =line;
        index++;
    }
    myfile.close();
}
else 
    cout << "Unable to open file" << endl; 

for(int i=0; i <12;i++)     {
    if(rows[i].find("EQUATIONS: ")!=string::npos)       { 
        i++;
        i++;
        while(i <index) { 
            arrForEquations[equationCount]=rows[i];
            equationCount++;
            i++;
        }
        break;
    }

    if(rows[i].find(':')!=string::npos)     {
        firstIntOccur =rows[i].find(':');
        tempForSize[i].assign(rows[i],firstIntOccur+2,rows[i].size());
    }
}

for(int  i =0;i <5; i++)        {  
    sizeArray[i] = atof(tempForSize[i].c_str());
}
try
{
    string * equations = arrForEquations;
    GeneticAlgorithm a(sizeArray, equations, equationCount);
}
catch(string s)
{
    cout << s << endl;
}

return 0;

 }

The constructor of the GeneticAlgorithm Class:

GeneticAlgorithm::GeneticAlgorithm(double *& arr, string * sArr, int size)      {
Equation ** e = new Equation*[size];
for(int i = 0; i < size; i++)   {
    e[i] = new Equation(sArr[i]);
}
}

The equation class works perfectly when strings are entered into it, I just can't figure out why this does not want to work.

Thanks in advance.

Upvotes: 0

Views: 156

Answers (2)

James Kanze
James Kanze

Reputation: 153909

This isn't really an answer to your question, but it will point out a lot of other mistakes, and once they've been cleaned up, it may become possible to understand what is actually taking place.

The first problem is that there is no functional decomposition. The function is just too big, and should be cut into several smaller functions. In general (and there are some notable exceptions), if a function is longer than about eight or ten lines, it needs refactoring.

On to more detailed problems: there are too many magic numbers, and for the most part, they are wrong. Your user will not input exactly 15 lines. He will input 5, or 500 or who knows. (If, of course, he enters 500, you've got a big problem.) The simplest and most common way of handling this is to use std::vector and its push_back member function. This isn't perfect (what happens if your user enters 5 trillion lines, or 1 line with 5 trillion characters), but for most appilications, it's good enough.

More generally, this applies to pretty much all of your arrays. Just use std::vector, and let them grow as needed, rather than using a magic constant for the size. And why on earth:

string tempForSize[5];
double * sizeArray = new double[5]; 

Why dynamic allocation in one case, and not in the other? (For that matter, you don't need tempForSize anyway. Once you've got the string value in a local temporary, you can convert it immediately and put it into the sizeArray.)

The loop while ( myfile.good() ) is never really correct. And you use the line read by getline without testing whether the read succeede or not. The correct way to do something like this is:

std::vector<std::string>
readRows( std::istream& source )
{
    std::vector<std::string> results;
    std::string line;
    while ( std::getline( source, line ) ) {
        result.push_back( line );
    }
    return results;
}

Rewrite the code using standard best practices, so that we can see what is actually going on, and we aren't distracted by so many other problems, and if you still have problems, then ask again.

(BYW: where on earth do people get the idea to loop on file.good(). It comes up over; if there's some textbook showing it, then we really have to act in some way to get it taken out of circulation.)

Upvotes: 1

Vaibhav Desai
Vaibhav Desai

Reputation: 2728

Quick note which does not answer your question but I still was compelled to point out. Avoid using pointers.

// Instead of using 'string * equations = new string[12]'
std::vector<std::string> equations;

// Instead of using 'double * sizeArray = new double[5]'
std::vector<double> sizeArray;

Use STL wherever you can. They are provided to make your life easy and the code looks less cryptic.

Upvotes: 2

Related Questions