Ask user for file name and open file. |
Strictly speaking, this needs two functions: a function should ideally "do one thing".
1 2 3 4 5
|
string fileName = AskUserForFileName();
// if successful...
OpenFile(fileName);
|
(though internally, you should be thinking in terms of file
paths rather than
names)
So saying, I think it would prob be better to leave this code (which asks the user for a file name and open the file) in main. I would also leave the ifstream's clear() and seekg() in main.
So I see 3 functions -- the last two in your list plus a helper method (for use by the function to write the shfted chars out)
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
|
char RotateCharLeft(char ch, int shift) {
if(islower(ch)){
ch -=shift;
if(ch < 'a'){
ch+=26;
}
}
if(isupper(ch)){
ch -=shift;
if(ch < 'A'){
ch +=26;
}
}
return ch;
}
|
When it comes to function parameters, the function to calculate the shift should probably have the signature
int func_name(istream& in) // using better names
You can (and should) use istream rather than fstream as your code uses no file specific functionity. This will allow your function to be reused with cin or an istringstream if you wish to do so.
Similarly, the signature for the output function should probably be
void func_name(istream& in, ostream& out, int shift) // using better names
used for the moment like
func_name(fromFile, cout, shift);
But reusable with an ofstream or ostringstream for the output, if you want.
Whatever function names you choose, they should make it clear what they do. So, to me, the name "counter" (which I take to be just for illustrative purposes in ace's post) is both a bit on the terse side and unclear (count what?). A common opinion is that as functions
do things, their names should reflect that: i.e. be a verb or verbal phrase. The name should preferably also make it clear what they're doing it to, etc.
(class getter and setter member functions exceptions. Tthey are sometimes just the name of the property they operate on (e.g. std::string::length), though others favour (e.g.) getLength() or similar.)
And while you're refactoring, you should also tighten up the scoping of your variables, for example:
- for loops
for(int index = 0; index < 26; index++) {
- declare variables at first point of use, or as near as possible
(ideally initializng variables it they aren't used immediately)
And consider using ifstream like this:
1 2 3 4
|
char ch = '\0';
while (is >> ch){
// use ch...
}
|
rather than using eof()
Andy
PS regarding:
Also, this code above is unsatisfactory in that it doesn't "text wrap" when it gets to the screen. Is there something from the iomanip library that can help? |
Sadly, std::istream knows nothing about text line wrapping. You'll have to sort out that yourself.