>int ndatamax=100;
>string line[ndatamax]
C++ requires array sizes to be constant.
>while(!myfile.eof())
*sigh* It seems we have to tell
everyone that this is the wrong way to use the eof member function. The eofbit is only set
after an attempt has been made to read from the stream and failed. This means that the body of your loop will execute once more than expected. For example:
#include <cstddef>
#include <fstream>
#include <iostream>
#include <string>
void create_file ( const char *filename, const char *contents )
{
std::ofstream out ( filename );
if ( out )
out<< contents;
}
int main()
{
const char *filename = "test.txt";
create_file ( filename, "Line1\nLine2\nLine3\n" );
{
std::ifstream in ( filename );
std::string line;
std::size_t n = 0;
if ( in ) {
while ( !in.eof() ) {
std::getline ( in, line );
++n;
}
std::cout<<"The file has "<< n <<" lines\n";
}
}
{
std::ifstream in ( filename );
std::string line;
std::size_t n = 0;
if ( in ) {
while ( std::getline ( in, line ) )
++n;
std::cout<<"The file has "<< n <<" lines\n";
}
}
}
Even worse, the content of the file will affect the result of the broken loop. If you change "Line1\nLine2\nLine3\n" to "Line1\nLine2\nLine3", the first loop will result in 3 instead of 4, but the second loop will still produce 3. The rule of thumb is that the eof member function (or feof in C) should never be used to drive an input loop. It's just too easy to screw up.
>ndata=i-1;
With your broken loop, this becomes an intermittent bug.
>for(int j=i+1;j<ndata;j++)
>if( line[i]>line[j]) { linetemp=line[i]; line[i]=line[j];line[j]=linetemp;}
Just for future reference, I think this formatting is evil. First, multi-line blocks should never be scrunched up onto a single line. Second, whitespace is your friend, so the conditional should look like this:
if ( line[i] > line[j] ) {
linetemp = line[i];
line[i] = line[j];
line[j] = linetemp;
}
Next, when you format the conditional in an easier to maintain fashion, the seemingly one line loop suddenly has five lines in the body:
for ( int j = i + 1; j < ndata; j++ )
if ( line[i] > line[j] ) {
linetemp = line[i];
line[i] = line[j];
line[j] = linetemp;
}
Just because a compound statement counts as a single statement doesn't mean that omitting braces for this loop is a good idea. In fact, not using braces is a pretty bad idea in general except when you have a very trivial single line body. That way it's immediately obvious that braces need to be added with more lines. This is much better:
for ( int j = i + 1; j < ndata; j++ ) {
if ( line[i] > line[j] ) {
linetemp = line[i];
line[i] = line[j];
line[j] = linetemp;
}
}
In fact, a good guideline to follow is that every block has braces.
Online 
Offline 
Unsolved 
