help with an assignement please

[loslos]

#include <iostream>
#include <fstream>
#include <cctype>
using std::fstream;

using namespace std;

int count[26];
int main(int argc, char *argv[])
{
  char ch;
  
  ifstream infile("c:\\a.txt") ;
  if ( !infile )
  {
        cerr << "File could not be opened" << endl;
        exit( 1 );
  }
  int i;
  for(i = 0; i <26; i++) count[ i ] = 0;
 
  while (! infile.eof())
  {
      ch = cin.get();
      if(isalpha(ch)) 
      {
          ch = toupper(ch); 
          count[ch-'A']++;
      }
  };
  for(i = 0; i <26; i++) {
    cout << (char) ('A'+ i) << ": " << count[ i ] << '\n';
  }

  return 0;
}

what i wanted to do with this code was input a stream of alphabetical characters and output the amount of each alphabetical letter. for ex. if the text said aaabbccccc.....yyzzzz i want to output
a:3
b:2
c:5 etc etc

but even this compiles alrigght and file was opened all i get is a blank screen. i'm really stumped and since i'm such a newbie i really don't know what im doing wrong. ang suggestions or comments will be useful.

[Ancient Dragon]

>> ch = cin.get();
you should use infine, not cin

ch = infile.get();

And thanks for taking the time to read the board's rules to use code tags correctly. Not many first-time posters do that.

[Lerner]

Off topic, but in the spirit of trying not to learn bad habits, and trying to avoid bugs that can be difficult to find when trying to determine why you don't get the results you expect on trial runs with known input as you test your code, the following

while (! infile.eof())
  {
      ch = cin.get();

could be changed to

while(infile >> ch)

You should avoid using the return value of eof() as the controlling condition for a loop. There are two reasons I know of to want to do this.

First, using the return value of eof() to terminate the loop is likely to cause a double counting of the last char in the file. If that char is a newline, then the double count will go unnoticed in your current code. But if the file terminates with an alphabetical char, it may, or may not, be counted twice. Inmy experience, trying to figure out why the code works appropriately sometimes and not others is probably the hardest bug to find.

Second, using the return value of the input statement to control the loop allows the loop to terminate appropriately, whether it terminates becuase it read the entire file, or whether it terminates because of some other cause. The loop might not terminate appropriately if the control condition is only whether the EOF marker was found but the linput stream fails for some other reason.

[~s.o.s~]

You should avoid using the return value of eof() as the controlling condition for a loop. There are two reasons I know of to want to do this.

First, using the return value of eof() to terminate the loop is likely to cause a double counting of the last char in the file. If that char is a newline, then the double count will go unnoticed in your current code. But if the file terminates with an alphabetical char, it may, or may not, be counted twice. Inmy experience, trying to figure out why the code works appropriately sometimes and not others is probably the hardest bug to find.

That would be feof( ) you are talking about which returns zero only *after* the end of file is reached and not *when* the end of file is reached. eof( ) is actually good and returns a boolean value when the end of file is reached.

[Ravalon]

That would be feof( ) you are talking about which returns zero only *after* the end of file is reached and not *when* the end of file is reached. eof( ) is actually good and returns a boolean value when the end of file is reached.

I don't get it. :confused: eof() returns true if the eofbit is set, but the eofbit doesn't get set until you hit end of file while reading. Unless you test for end of file after the read, the order is wrong and you'll loop one too many times. Here's an example that shows the problem.

#include <fstream>
#include <iostream>
#include <string>


int main()
{
  using namespace std;

  // Build a test file
  {
    ofstream os( "test.txt" );

    os << "12345";
  }

  // Test the test file :)
  ifstream is( "test.txt" );
  int loopCount = 0;
  char ch;

  while ( !is.eof() ) {
    ++loopCount;
    is.get( ch );
    cout << ch << '\n';
  }

  cout << "Loop count: " << loopCount << '\n';

  return 0;
}

eof() and feof() have the same problem because they work the same way.

That would be feof( ) you are talking about which returns zero only *after* the end of file is reached and not *when* the end of file is reached. eof( ) is actually good and returns a boolean value when the end of file is reached.

No I believe eof has similar problems when the file contains extra newlines.

Like lerner and ravalon said best to avoid it altogether, especially in c++. I'm surprised your little google groups didn't tell you that sos. Ha ha.

[~s.o.s~]

I don't get it. :confused: eof() returns true if the eofbit is set, but the eofbit doesn't get set until you hit end of file while reading. Unless you test for end of file after the read, the order is wrong and you'll loop one too many times.

My bad, I should have been more explicit. Yes, I agree, feof( ) and eof( ) are not that great when compared to checking the stream state while reading files. But for small fixes I normally use:

#include <fstream>
#include <iostream>
#include <string>

int main()
{
  using namespace std;

  // Build a test file
  {
    ofstream os( "test.txt" );

    os << "12345";
  }

  // Test the test file :)
  ifstream is( "test.txt" );
  int loopCount = 0;
  char ch;

  while (1 ) {
    is.get( ch );
    if( is.eof() ) break ;
    ++loopCount;
    cout << ch << '\n';
  }

  cout << "Loop count: " << loopCount << '\n';
  getchar( ) ;
  return 0;
}

I'm surprised your little google groups didn't tell you that sos. Ha ha.

*sigh* Btw when are you turning 14 ?

[Dave Sinkula]

But for small fixes I normally use:

while ( 1 ) {
    is.get( ch );
    if( is.eof() ) break ;
    ++loopCount;
    cout << ch << '\n';
  }

Why that instead of this? Seems like more for less.:?:

while ( is.get( ch ) ) {
    ++loopCount;
    cout << ch << '\n';
  }

This will break the loop on errors too.

Yes, I agree, feof( ) and eof( ) are not that great when compared to checking the stream state while reading files. But for small fixes I normally use:

As always, in these circumstances go for the solution that works every time. Sure you can build functionality in eof() so that it works correctly.

Hell you can even use gets() safely if you go outta your way to implement the requisite error checks.

But that's extra work for very little benefit.

See you don't learn this at the google groups.:lol:

[~s.o.s~]

As always, in these circumstances go for the solution that works every time. Sure you can build functionality in eof() so that it works correctly.

Hell you can even use gets() safely if you go outta your way to implement the requisite error checks.

But that's extra work for very little benefit.

The code snippet / my post was meant to illustrate that using eof( ) is not completely wrong or taboo if used the right way. Just ignoring a part of language just because using it in the wrong way ensues chaos is foolishness.

An use of eof( ) would be:

char buf[BUFSIZE];
 do {
   in.read( buf, BUFSIZE );
   std::streamsize n = in.gcount();
   out.write( buf, n );
 } while( in.good() );
 if( in.bad() || !in.eof() ) {
   // fatal error occurred
 }
 in.close();

Not bad, is it...

So stop considering a feature bad just because someone says so...

See you don't learn this at the google groups.

I guess you were badly flamed at google groups for obvious reasons, thats why you keep mentioning it.....

[Dave Sinkula]

An use of eof( ) would be:

char buf[BUFSIZE];
 do {
   in.read( buf, BUFSIZE );
   std::streamsize n = in.gcount();
   out.write( buf, n );
 } while( in.good() );
 if( in.bad() || !in.eof() ) {
   // fatal error occurred
 }
 in.close();

Why discard the good/bad result and then call another function later to get a good/bad result? That is, why not this?

char buf[BUFSIZE];
   while ( in.read( buf, sizeof buf ) )
   {
      out.write( buf, in.gcount() );
   }
   if ( in.bad() || !in.eof() )
   {
      // fatal error occurred
   }

[Ravalon]

Hell you can even use gets() safely if you go outta your way to implement the requisite error checks.

I don't think there's a way to use gets() safely. All of the damage is done behind the scenes where error checks won't help.

The code snippet / my post was meant to illustrate that using eof( ) is not completely wrong or taboo if used the right way. Just ignoring a part of language just because using it in the wrong way ensues chaos is foolishness.

Nobody said to ignore it, just that there may be problems if you use it that way. ;) I think you reacted before fully absorbing what we've been saying and took our advice to the illogical extreme. :sad:

[WaltP]

Nobody said to ignore it, just that there may be problems if you use it that way. ;)

Exactly. No one said eof() was bad. They just said while (!xx.eof()) is the wrong way to use it. That form won't work in most cases. You used it properly. The OP didn't.

[Ancient Dragon]

char buf[BUFSIZE];
 do {
   in.read( buf, BUFSIZE );
   std::streamsize n = in.gcount();
   out.write( buf, n );
 } while( in.good() );
 if( in.bad() || !in.eof() ) {
   // fatal error occurred
 }
 in.close();

One problem with the above code is that when end-of-file is reached and write() will be executed with 0 data to write, such depending on implementation, might in worse case cause an exception.

I agree with the others that its best to avoid using eof() or feof(). There are other more efficient ways to code a loop like that.

[loslos]

hey guys thanks for all the help especially the first two posters. when adding in there suggestions i was able to get a display of values. only trouble is that for some reason half the values are missing. for ex. if there were 10 a's in the text, the output says theres only 5.

#include <iostream>
#include <fstream>
#include <cctype>
using std::fstream;

using namespace std;

int count[26];
int main(int argc, char *argv[])
{
  char ch;
  
  ifstream infile("c:\\a.txt") ;
  if ( !infile )
  {
        cerr << "File could not be opened" << endl;
        exit( 1 );
  }
  int i;
  for(i = 0; i <26; i++) count[ i ] = 0;
 
  while(infile >> ch)
  {
      ch = infile.get();
      if(isalpha(ch)) 
      {
          ch = tolower(ch); 
          count[ch-'a']++;
      }
  };
  for(i = 0; i <26; i++) {
    cout << (char) ('a'+ i) << ": " << count[ i ] << '\n';
  }

  return 0;
}

heres the code with the small changes. ive read all the posts but since im pretty slow don't really understand it. anyways thanks a bunch to everyone.

[Ravalon]

hey guys thanks for all the help especially the first two posters. when adding in there suggestions i was able to get a display of values. only trouble is that for some reason half the values are missing. for ex. if there were 10 a's in the text, the output says theres only 5.

Your loop condition also does double duty as input. But you throw that value away and read the next one in the loop body.

while(infile >> ch) // Read one character
{
    ch = infile.get(); // Read another character

The effect is that you're ignoring every other character and cutting your results in half. You can fix it by removing the second input, ch = infile.get(); . :)

[loslos]

thank for all the help dragon. it's alotta small things things here and there that are screwing me up.
i've wanted some other help with that particular assignment as well. after doing that i need to make a horizonetel histogram of the data using = after each number

#include <iostream>
#include <fstream>
#include <cctype>
using std::fstream;

using namespace std;

int count[26];
int main(int argc, char *argv[])
{
  char ch;
  
  ifstream infile("c:\\a.txt") ;
  if ( !infile )
  {
        cerr << "File could not be opened" << endl;
        exit( 1 );
  }
  int i;
  for(i = 0; i <26; i++) count[ i ] = 0;
 
  while(infile >> ch)
  {
       if(isalpha(ch)) 
      {
          ch = tolower(ch); 
          count[ch-'a']++;
      }
  };
  for(i = 0; i <26; i++) 
  {
    cout << (char) ('a'+ i) << ": " << count[ i ] << '\n';
  }
  for(i = 0; i <26; i++) 
  {        
      int length;
      int j;
      length = ((count[i] * 80)/(float)394);
      for (j = 0; j < length; i++)
      {
          cout << "=" << endl;
      }

  return 0;
  }

}

the last part is where im dumbingly trying to add it to my output. all that gets prints out is = signs that seem to loop forever. any help would be great guys.

[~s.o.s~]

There is acutally a typo in your program -- in your second loop while displaying, you have wrongly used and incremented the variable "i" when it should have been j. The result being that "j" never gets changed and results in an infinite loop.

Some tips:
• Avoid forward declaring the loop control variables since its better if you declare and initialize the loop control variable in the loop itself.

for( int i = 0; i < SIZE; ++i ) { // code }

• Your floating casting which happens in the second display loop is superfluous since it has no effect -- the values are in the end rounded off to integers. Better come up with a integer scaling factor which would greatly simplify the problem rather than coming up with cryptic calculations.

• Avoid delaring variables in loops. Though modern compilers have become really smart and perform a lot of optimizations, its better not to test its patience and write good code. Peform varaiable declarations just before the point of using them.

Something like:

for(int i = 0; i <26; i++)
{
      length = count[i] ;
      cout << static_cast<char>('a' + i) << ":  " ;
      for (int j = 0; j < length; j++)
      {
          cout << "* " ;
      }
      putchar( '\n' ) ;
}

[WaltP]

for(i = 0; i <26; i++) 
  {        
      int length;
      int j;
      length = ((count[i] * 80)/(float)394);
      for (j = 0; j < length; i++)
      {
          cout << "=" << endl;
      }

  return 0;
  }

}

the last part is where im dumbingly trying to add it to my output. all that gets prints out is = signs that seem to loop forever. any help would be great guys.

Look carefully at your 2 for loops... Do you see a mismatch of variables?

And is your return where you want it?

[loslos]

ugh ok this is the reason why i cannot be a programmer. j instead of an pshhhhhhhhh. ya i used

for(i = 0; i <26; i++)//find maximum value in array results
          if(count[i]>i)
                  max = count[i];

 for(i = 0; i <26; i++)
 {
         length = count[i]*74/max ;//scales the bar for histogram

still put the 74 cuse it holds constant no matter what the text is (i think)
and yah the return made it so it wouldnt go to each letter but print only a.
ok thanks for the all help guys. took like 6 people to conquer this thing........if only the world was this easy.