Trouble with file pointer

Quote:

---

Originally Posted by nirvana21

1. Why does it matter that the '&' symbol is connected to ifstream rather then fptr?

---

That's just the thing - it doesn't. The compiler doesn't care. It's more of a style thing. Bjarne had a question about that so I will link it: http://www.research.att.com/~bs/bs_faq2.html#whitespace
In C++, there is a heavy emphasis on types, which is why I always put them next to the type.

Quote:

---

2. Why do I need to say std::, I thought saying 'using namespace std' allowed me to not type that?

---

You're right, you do not. I always like to prefix them, though.

Quote:

---

3. I saw .c_str() used in my book but it was unclear whats it purpose is. I also saw .data() and I am currently using that. What is the difference between the two and what is the purpose of the () at the end of them?

---

c_str() returns a constant C-style string of the contents inside the string class.
The () at the end is because it's a function, and not a member variable. Function calls always needs the ().

Quote:

---

4. It is not required but a good idea? Sort of like a comment in the code?

---

Did you not read the link I linked to? I think it explains very well why it's usually a bad idea to strip them.

Quote:

---

Originally Posted by nirvana21

1. Why does it matter that the '&' symbol is connected to ifstream rather then fptr?

---

Lexically, it does not matter. The whitespace is not significant at that point. Semantically, it can matter, because it can be read as "fptr is of type ifstream&" (reference to ifstream).

Quote:

---

Originally Posted by nirvana21

2. Why do I need to say std::, I thought saying 'using namespace std' allowed me to not type that?

---

In that case you do not need to fully qualify names in the std namespace, but you can still do so, especially if it is needed to disambiguate names.

Quote:

---

Originally Posted by nirvana21

3. I saw .c_str() used in my book but it was unclear whats it purpose is. I also saw .data() and I am currently using that. What is the difference between the two and what is the purpose of the () at the end of them?

---

c_str() returns a pointer to the first character of a null terminated string corresponding to the data of the string object itself. data() returns a pointer to the same, except that the string is not guaranteed to the null terminated.

I just want to say that this forum has really helped me with this program. Thanks to all of you. Maybe later I will post my final program to be critiqued.

:cool:

Here is my final program. Please critique if you feel like it. I am new to C++ so tell me how it is.

Code:

---

#include <iostream>
#include <iomanip>
#include <fstream>
#include <string>
       
using namespace std;
                       
               
void welcome (void);
                       
void getfile (ifstream &);
               
void character_processor (ifstream &, int []);
       
void display_results (int []);
       
       
int main (void){
       
        int counts[27]={0};
       
        ifstream fptr;

        welcome();
       
        getfile(fptr);
       
        character_processor(fptr,counts);

        display_results(counts);

        return(0);

}
       

       
void welcome(void){

        cout << "\n\nLetter Counter\n\nWelcome to the letter counter. This ";
        cout << "program will prompt you for an input file of\ntext. It ";
        cout << "will read the text and report the number of times each ";
        cout << "letter of the\nalphabet is used. It will also display a ";
        cout << "visual graph.\n\n";

        return;
       
}     
       
void getfile(ifstream& fptr){
       
        string filename;
       
        for(;;){
       
                cout << "Please provide filename for input file: ";
       
                getline(cin, filename);
       
                fptr.open(filename.c_str(),ios::in);
                       
                if (fptr.is_open())

                        return;

                else
       
                        cout << "\nError opening file!\n\n";
       
        }
       
}
       
void character_processor(ifstream& fptr, int letters[]){
       
        char data;

        int temp;
       
        while(!fptr.eof()){
       
                data = fptr.get();

                temp = (int)data;
               
                if(temp >= 97)
               
                        temp = temp - 32;
               
                temp = temp - 65;
                       
                if((temp >= 0) && (temp < 26)){
               
                        letters[temp]++;
                       
                        letters[26]++;
       
                }
 
        }

        return; 
       
}
       
       
void display_results (int letters[]){
       
        char letter;

        int whole_percent;
               
        cout << "\n\n" << "Letter  Occurrances  Percent Graph\n\n";
               
        for(int i=0; i<=25 ; i++){
               
                letter = i + 65;
                       
                cout <<"  "<<letter<<"        "<<setw(5)<<right<<letters[i];
               
                cout<< "      ";
                       
                whole_percent = (((double)letters[i]/letters[26])*100);
       
                for(whole_percent; whole_percent >= 0; whole_percent--){
 
                        cout << "*";

                }
       
                cout << endl;
       
        }

        cout << "\n\nThe total number of letters is " << letters[26];
       
        cout << ".\n\n";
       
        return;
       
}

---

It is a matter of style, but personally I feel that you are leaving too many blank lines. I would rather leave a blank line when I want to separate one group of statements from the next, but then this grouping can be rather subjective as well.

Consider this function:

Code:

---

void character_processor(ifstream& fptr, int letters[]) {
    char data;
    int temp;

    while (!fptr.eof()) {
        data = fptr.get();
        temp = (int)data;

        if (temp >= 97)
            temp = temp - 32;
        temp = temp - 65;

        if ((temp >= 0) && (temp < 26)) {
            letters[temp]++;
            letters[26]++;
        }
    }

    return;
}

---

One problem is the use of eof() to control the loop. Since eof is only set after a failed read, the loop goes through more one iteration than it should. Another problem is the use of magic numbers. I presume that 97 should be 'a' and 65 should be 'A'. Likewise, letters[26]++ looks like a bug, but it turns out that you are using it to record the total count, so perhaps you should use a named constant instead of 26 to make this obvious. Also, you only need one local variable, not two. As such, I might write it as:

Code:

---

enum { total_count_index = 26 };

// ...

void character_processor(ifstream& fptr, int letters[]) {
    int index;

    while ((index = fptr.get()) != EOF) {
        if (index >= 'a')
            index -= 32; // Change lower case to upper case
        index -= 'A';

        if ((index >= 0) && (index < 26)) {
            ++letters[index];
            ++letters[total_count_index];
        }
    }
}

---

Quote:

---

One problem is the use of eof() to control the loop. Since eof is only set after a failed read, the loop goes through more one iteration than it should.

---

So this doesn't cause an error, its just inefficient?

Code:

---

++letters[index];

---

This increases the variable beforehand correct? So really what I had would have worked but this is sleeker?


Thanks

Quote:

---

Originally Posted by nirvana21

So this doesn't cause an error, its just inefficient?

---

Yes, it does not cause an error because you check the range of the character read later on.

Quote:

---

Originally Posted by nirvana21

This increases the variable beforehand correct? So really what I had would have worked but this is sleeker?

---

Since the expression is standalone it results in the same thing, but you should prefer the prefix form because in general it is no less efficient, and may be more efficient (just not in this case).

Quote:

---

Originally Posted by nirvana21

So this doesn't cause an error, its just inefficient?

---

Not only that, but it's dangerous.
You will most likely double data on the last line, because it will try to read a new line, fail, and thus the previous data is left inside the buffer.

Quote:

---

Code:

---

++letters[index];

---

This increases the variable beforehand correct? So really what I had would have worked but this is sleeker?

---

Yes, it does. But it's not necessarily sleeker; it's a matter of style. With primitive types such as int, when you are only doing incrementation, it doesn't matter if you do ++ before or after. With classes, generally it's more efficient to do it first unless you have a need not to.
It's a matter of style.

I am having a rounding problem as well. I thought that a number that is converted from double or float to an int would round up. Example: double .001 = int 1. This must not be the case because I am having trouble here:

Code:

---

whole_percent = (((double)letters[i] / letters[total_count_index]) *100);

                for(whole_percent; whole_percent > 0; whole_percent--){
               
                        cout << "*";
                       
                }

---

If I have a large text file and a letter only shows up very few times, I do not get an asterisk as output. Is there an easy way to address it or should I do add something like this:

Code:

---

if((letter[i] > 0) && (whole_percent == 0)){
               
        whole_percent = 1;
                       
}

---

Quote:

---

Originally Posted by nirvana21

I thought that a number that is converted from double or float to an int would round up. Example: double .001 = int 1.

---

No, it would be truncated (round down).

Quote:

---

Originally Posted by nirvana21

I am having a rounding problem as well. I thought that a number that is converted from double or float to an int would round up. Example: double .001 = int 1.

---

Other way. .001 -> 0; .005 -> 0; .999 -> 0. double to int always truncates.