Hi! Still being an absolute beginner, I can't find a reason for why this program doesn't run properly (note that the program is not complete). It is possible to compile it but it encounters some kind of error when executing. The string saves the file properly I have checked this by using cout << so I guess the error is in the class definition, no? I would be truly grateful if you could help me somehow so I can grasp the concept of files becoming strings going through object methods.

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

const int NUMBER_OF_LETTERS = 26; // The alphabet

class Text
{
private:
  string textstring;
  int histogram_abs[NUMBER_OF_LETTERS];
  double histogram_rel[NUMBER_OF_LETTERS];
  int letters_total;

public:
  Text();    
  ~Text();   
  
  void setText(string text);
  bool calculateHistogram(); 
  
};


string file_name(string &text);

void read_file(string &rad, string &text);



int main()
{
  string text, rad;
  bool histOK;
  Text myText;  
  
  file_name(text);
  read_file(rad, text);
  
  myText.setText(text);
  histOK = myText.calculateHistogram();
  
  system("pause");
  return 0;
}

Text::Text()
{
  textstring="";
  for(int i=0;i<NUMBER_OF_LETTERS;i++)
     {
      histogram_abs[i]=0;
    }
  for(int i=0;i<NUMBER_OF_LETTERS;i++)
    {
      histogram_rel[i]=0.0;
    }
  letters_total=0;
}

Text::~Text() 
{
}

void Text::setText(string text)
{
  textstring=text;
}

bool Text::calculateHistogram()
{
 for(unsigned int i = 0; i<textstring.length(); i++)
   {
     if(isalpha(textstring[i]))
       letters_total++;
     if(isupper(textstring[i]))
       textstring[i]=tolower(textstring[i]);
     histogram_abs[(textstring[i] - 'a')]++;
    }

  if(letters_total==0)
    return false;
  else
    return true;
  
}

string file_name(string &text)
{
       cout << "File name:\n";
       cin >> text;
       int john=text.rfind(".txt");
       if (john == string::npos)
       text.append(".txt"); 
       return text;
}

void read_file(string &rad, string &text)
{
     ifstream fin(text.c_str() );

     if(!fin)
     {
             cout << "No file with that name in current folder. " << text << endl;             
     }
while(!fin.eof()) 
{
  string radtemp;  
  getline(fin,radtemp); 
  rad += radtemp;     
}
text=rad;
}

Actually, I just realised that this program manages to read shorter strings. I need it to be able to read quite long strings, though. Should I perhaps try changing some data types?

>> !fin.eof()

I think the error is here.The eof() is only a one condition.
!file.eof()&& !file.fail() && !file.bad()

please don't forget to optmize this code using demorgan's rule.

Thank you! However, this is beyond my knowledge so I'm still stuck with this. I will probably need to read at lot to get into this. Would be great to get rid of this one obstacle, though. I've tried using the eof() condition once before, successfully. Do you suggest me to replace it with the

!file.eof()&& !file.fail() && !file.bad()

line?

best,
John

I just realised what is wrong, but I'm unable to make it work. The problem is that, if the text file contains a numbers, the program won't execute properly. Does anyone know how to get around this problem, by ignoring the numbers in the file when reading it into a string in order to calculate it?

You have a file. You have variables that you have to read that file into. Your job is to write code that matches the file layout and the variables. getline and eof() are tools. Depending on the file layout, they may or may not be the right tools. If you are a beginner, you may not yet have a firm grasp of what tools to use for what purpose.

getline "gets a line" and throws away the newline. It is used for strings. If you don't have a string variable, either don't use getline or use it to read to a string, then convert the string to something else. If you are looking to read in an integer, you can use getline, then convert to a C-Style string using c_str(), then use the atoi from cstdlib or something similar, or you can read into an integer directly using >>.

If you have a file and you want to extract the letters and nothing but the letters and put them all in a big string, don't use getline or eof(). I mean you could, but it's extra work. Use the plain old >> operator.

string allLetters = "";
char temp;
while(fin >> temp)
{
   if (isalpha(temp))
   {
       allLetters += temp;
   }
}

You may need to elaborate a little more about exactlyt what's supposed to end up in this long string.

Here is a problem here:

if(isalpha(textstring[i]))
    letters_total++;
if(isupper(textstring[i]))
    textstring[i]=tolower(textstring[i]);
histogram_abs[(textstring[i] - 'a')]++;

Adding in brackets:

if(isalpha(textstring[i]))
{
    letters_total++;
}
if(isupper(textstring[i]))
{
    textstring[i]=tolower(textstring[i]);
}
histogram_abs[(textstring[i] - 'a')]++;

Note that the last line executes regarless of either if test. Almost certainly you do not want this. Suppose the character is '0', which is 48 in ASCII. I forget what 'a' is, but it's more than 48, which means that '0' - 'a' is a negative number, which crashes the program. You need to add some more testing/handling for non-letters. Try adjusting the brackets above to make sure.

And now is a good time to get familiar with the assert command from the cassert library.

if(isalpha(textstring[i]))
{
    letters_total++;
}
if(isupper(textstring[i]))
{
    textstring[i]=tolower(textstring[i]);
}
int index = textstring[i] - 'a';
assert(index >= 0);
histogram_abs[index]++;

Run it with this change and you'll see EXACTLY where the program bombs and debug from there.

Thanks for all your help! I'm sure I will understand it properly some day :) I was able to execute the program by changing the following...

if(isalpha(textstring[i]))
{
    letters_total++;
}
if(isupper(textstring[i]))
{
    textstring[i]=tolower(textstring[i]);
}
histogram_abs[(textstring[i] - 'a')]++;

INTO

if(isalpha(textstring[i]))
{
    letters_total++;
}
if(isupper(textstring[i]))
{
    textstring[i]=tolower(textstring[i]);
histogram_abs[(textstring[i] - 'a')]++;
}

BUT it calculates the wrong result. for example, the program thinks that there are 116 i-characters but only 5 e-characters in the text, which is not correct as the text contains many e:s. However, I was able to execute it so that's some kind of progress.

As for the other advice it's all fascinating but I'm not able to use it as the program won't compile. I simply don't know where to put the code in order to make it work.

Edited 6 Years Ago by John Sand: n/a

Suppose textstring an STL string object and the content of the string is "My 7th program". The goal is to calculate how many of each letter there is in the string by looking at each char in textstring one at a time:

int len = textsting.length();  //determine the length of the string
for(int i = 0; i < len; ++i) //look at each char in the string
{
  if this char is alphabetical letter
  {
    increment total number of letters found
    if this char is upper case
    {
      convert this char to lower lower case
    }
    increment the counter varible for this letter.
  }
  //else 
    //this char isn't a letter so ignore it.
}

Note how you can nest one if statement within the other so you only run the second if statement if the first one is true.

Assuming the counter variables are set to zero before you use them in this snippet it shouldn't matter what all is in textstring or where/how you get textstring.

if(isalpha(textstring[i]))
{
    letters_total++;
}
if(isupper(textstring[i]))
{
    textstring[i]=tolower(textstring[i]);
histogram_abs[(textstring[i] - 'a')]++;
}

Better, but still a bit of a problem. If I'm understanding the goal, the whole point is that an 'E' is to be treated as an 'e' (i.e. a letter is tallied regardless of its case). Go through the code above and see if it works on both lower and upper case.

Change your input file and make it nice and easy:

a
A
b
B
B
a
a
A
c
c
C

Get it so you get the desired results. Throw in a few change-ups till its all right.

As an added bonus, it's ofter very helpful to, before doing any processing, simply regurgitate the file that you've read in to prove to yourself that you read it in correctly.

As far as the other code I posted, yes, it's a skeleton to illustrate a point and won't compile in your code without some modification. Anyway, you must first isolate whether the problem is reading the file or whether it's the letter tallying. If it's the former, post a simple input file with the corresponding correct output so we can run it.

Edited 6 Years Ago by VernonDozier: n/a

I think the proper logic for the if-statements is this:

if(isalpha(textstring[i]))   //if it is a letter (not a number or other crap)
{
  letters_total++;           //increment letter count.
  if(isupper(textstring[i])) //if upper-case,
  {
    textstring[i]=tolower(textstring[i]); //then make it lower case.
  }
  histogram_abs[(textstring[i] - 'a')]++; //at this point, only a lower-case letter could be in textstring[i].
}

As it was before, only the capital E would be counted and only capital I (which is obviously more common in English).

Comments
Great advice on C++ programming.

Yes! It finally works, perfectly, and with such a small change. Basically, all that was need was for the

hist_abs[(textkonst[i] - 'a')]++;

to be in the first if-statement rather than the second, like this:

if(isalpha(textstring[i]))
{
    letters_total++;
histogram_abs[(textstring[i] - 'a')]++;
}
if(isupper(textstring[i]))
{
    textstring[i]=tolower(textstring[i]);
}

Thanks everyone for all your helpful information and your time, I am sure I will find it useful again and again!

Best,
John

Special thanks to Mikael on the letter dilemma etc. However, I was not able to use the code you just posted, but I probably did something wrong there. The logic behind it, and other info given here has made me understand this a lot better!

This article has been dead for over six months. Start a new discussion instead.