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];
}
}
}
Originally Posted by nirvana21
