File output problem

[kethgr]

Hello everyone.
I'm trying to write a program that decodes a message using the Caesar cipher (which I'm guessing a lot of you are familiar with). It's supposed to be able to decrypt just about any message, based on the assumption that the most common letter in English is 'E.'
My program compiles successfully, creates the output file with the correct name, and exits successfully, but when I open the output file afterward, it's empty.
I'm very new to programming and I can't tell what's wrong. I know I'm pretty shaky on functions, so the problem could be there. I thought I was going along pretty well, so hopefully my code is not a total mess. :/
Here's what I have:

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

using namespace std;

void characterCount(char ch, int list[]);
void initialize(int list[]);
void writeOutput(ifstream &in, ofstream &out, int shift);

int main()
{
	ifstream inFile;
	ofstream outFile;
	string inFileName, outFileName, reply;
	char character;
	int letterCount[26];
	int index, maxIndex = 0;
	int mostFreqLetter;
	int gap;
	
	initialize(letterCount);

	cout << "Please enter the name of the input file: ";
	getline(cin, inFileName);
	inFile.open(inFileName.c_str());
	cout << endl;

	if (!inFile.is_open())
	{
		cout << "Unable to open input file." << endl;
		cout << "Press enter to quit.";
		getline(cin, reply);
		exit(1);
	}
	
	cout << "Please enter the name of the output file: ";
	getline(cin, outFileName);
	outFile.open(outFileName.c_str());
	cout << endl;

	while (!inFile.eof())
	{
        	inFile.get(character);
		characterCount(character, letterCount);
	}

	for (index = 1; index < 26; index++)
		if (letterCount[maxIndex] < letterCount[index])
			maxIndex = index;
	mostFreqLetter = letterCount[maxIndex];

	gap = mostFreqLetter - static_cast<int>('E');

	writeOutput(inFile, outFile, gap);

	inFile.close();
	outFile.close();

	return 0;
}

void initialize(int list[])
{
	int j;
	for (j = 0; j < 26; j++)
		list[j] = 0;
}

void characterCount(char ch, int list[])
{
	int index;

	ch = toupper(ch);
	index = static_cast<int>(ch) - static_cast<int>('A');

	if (0 <= index && index < 26)
		list[index]++;
}

void writeOutput(ifstream &in, ofstream &out, int shift)
{
	char ch;
	int newCh;

	while (!in.eof())
	{
        	in.get(ch);

		newCh = static_cast<int>(ch) - shift;

		out << static_cast<char>(newCh);
	}
}

Any guidance you could give me would be great. Thank you!

[cire]

        inFile.clear() ;
        inFile.seekg(0, ios::beg) ;
	writeOutput(inFile, outFile, gap);

Your writeOutput function should loop on in.get(ch) not on !in.eof().

In the same way:

	while (!inFile.eof())
	{
        	inFile.get(character);
		characterCount(character, letterCount);
	}

should be:

	while (inFile.get(character))
	{
		characterCount(character, letterCount);
	}

[kethgr]

What does that first bit of code do? I don't recognize it from my textbook. :)

I made the other changes, and now the output file fills up with a bunch of weird symbols, none of which are numbers or Roman letters.

The weird thing is, the way I wrote the program I meant for it to leave everything except letters alone, but the output file doesn't even contain any of the numbers or punctuation from the input file. Any ideas?

I'm loving programming so far, but it gets really frustrating being a beginner and having so little instinct built up...
Anyway, thanks for your help!

[TheBeardedQuack]

I'm sorry i cant answer you're question but part of your code may be able to help me.
I am aware that the system() function and file.open() functions don't usually accept strings variables but i noticed you added .c_str() onto the end of a string in the file.open function.

Does this .c_str() function enable a string to be read in system() and file.open() ???

If this is the case then you've unknowingly but greatly helped me with one of my current projects!

[cire]

What does that first bit of code do? I don't recognize it from my textbook. :)

referring to:

        inFile.clear() ;
        inFile.seekg(0, ios::beg) ;

Clears the state of the stream. (In this case, you previously read to end of file and the eof bit will be cleared so the stream no longer thinks it is in an error state.)

inFile.seekg(0, ios::beg) moves the get pointer to the beginning of the file. If you didn't seek to the beginning of the file, the eof bit would be set immediately on your next attempt to read.

[cire]

On line 51 you set mostFreqLetter to the number of times the letter occurred.

Should be:
mostFreqLetter = maxIndex + 'A' ;

Note there's no need for casts.

[Lowest0ne]

You don't have to do any of those static_casts, char is already an integer.

Is your file totally empty, or is it perhaps a bunch of spaces (non-printable characters).

At line 51, mostFreqLetter equals the position in the alphabet of the max letter. Subracting it by 'E' doesn't make sense (right?), you need to subract it by 5 (the position of 'E" in the alphabet.

[kethgr]

@SatsumaBenji: I'm really new to programming, so I'm not sure about system() functions, but it's my understanding that adding .c_str() onto your file.open() does allow you to use strings with that command. Someone please correct me if I'm wrong.

@cire: Thank you for clarifying those. I have no prior programming experience and am taking this class online...I think this was a bad idea. I really appreciate all the help I've gotten from this forum. Another question: if mostFreqLetter is of type char, will maxIndex + 'A' yield whatever letter corresponds to the ascii code for the number in maxIndex + 65? I probably don't really understand how to properly use casts or how int and char types interact.

@LowestOne: It's no longer empty, it's full of junk. :) You're right, line 51 makes no sense as it is. I got confused by a combination of not knowing what I'm doing and making poor variable name choices.

I think I'm making progress here, but any more advice you have would be greatly appreciated. Thanks everyone!

[Lowest0ne]

Both char and int are integers, no decimal places. The difference is that char is one byte large and int is four bytes large. "int" can hold larger numbers. It is perfectly correct that c equals three at the end of this code:

char a = 1;
char b = 2;
char c = a + b;

But a char can can only store -128 through 127, so it's not really useful for math.

As for variable names, it is helpful to keep them the same through out the code. For example, I'm looking at the write function, wondering where the heck "shift" came from. If this were also gap, it might be easier for you.

[kethgr]

That's good to know. My book doesn't say anything about char being able to hold numerical values and always uses casts to go between int and char; but then I think it's really geared toward beginners and wants to avoid confusion.

It also advises not using the same variable names in different functions (unless I understood it wrong), which is why gap in main corresponds to shift in writeOutput. Is it better to use the same variable names in every function that refers to the same value?

[TheBeardedQuack]

If you're wanting to edit the exact same variable from multiple functions you should make the variable global (declare it outside a function) or refer to it specifically with it's exact address.
Otherwise you're making a new variable of the same name just for that specific function and this variable no-longer exists once you exit that function.

(this any help?)

[kethgr]

Yes, that makes sense to me, but it's inconsistent with what my textbook seems to be telling me. It says to avoid declaring variables at file scope unless you really know what you're doing. Since I clearly don't know what I'm doing, I've just been declaring variables within each function. The examples in my book almost always use different names for formal parameters and actual parameters.

Interesting update:
I've figured out that the file my program is supposed to decode is the Declaration of Independence. I can tell because now my program decrypts about half the characters in the file, but a lot of them are still wrong and there are way too many numbers.

I changed my writeOutput function as shown below and I'm guessing that's where the problem is. The characters need to "wrap" at Z and z, and maybe I haven't set it up correctly to do that? Thoughts?

void writeOutput(ifstream &in, ofstream &out, int shift)
{
  char ch;
  char newCh;

  while (in.get(ch))
  {
  if (ch > 64 && ch < 91)
    newCh = ch - shift;
  else if (ch > 96 && ch < 123)
    newCh = ch - shift;
  else
    out << ch;

  if (newCh < 65)
    out << newCh + 26;
  else if (newCh > 90 && newCh < 97 && ch < 91)
    out << newCh - 26;
  else if (newCh > 90 && newCh < 97 && ch > 96)
    out << newCh + 26;
  else if (newCh > 122 && ch < 96)
    out << newCh - 26;
  else
    out << newCh;
  }
}

[cire]

You should avoid the magic numbers.

void writeOutput(std::ifstream &in, std::ofstream &out, int shift)
{
	char ch;

	while (in.get(ch))
	{
		if ( isalpha(ch) )
		{
			// toupper and tolower return an int.  Using a typecast
			// to suppress a compiler warning.

			bool wasLower = islower(ch) ;
			if ( wasLower )
				ch = static_cast<char>(toupper(ch)) ;

			ch -= shift ;
			if ( ch < 'A' )
				ch = 'Z' - ('A'-ch) ;

			if ( wasLower )
				ch = static_cast<char>(tolower(ch)) ;
		}

		out << ch ;
	}
}

[Lowest0ne]

The useful thing about functions is that you can copy/paste them into other projects. Name your variables in the function to what makes the most sense. You know, ofstream& out, or perhaps ofstream& outObj.

Later, you might have a function that takes a variable "radians". From that perspective, when you need that function, you probably have a radian your working with. The variable's name might as well be radian, or radianX.

For basic info about functions:
http://www.cplusplus.com/doc/tutorial/functions/
For more:
http://www.cplusplus.com/doc/tutorial/functions2/

also to note that if you don't want to use more memory in your function, but you also want to be sure you don't modify the value of the variable:
void function(const int& a);

const makes it constant (not modifiable), and & makes it a reference to a memory address. You can use them one at a time too (const int a) or (int& a)

[kethgr]

Aaaah! This program's starting to drive me a little nuts.

cire, I replaced my old writeOutput function with the one you posted, and I'm getting the same result as before, and it's totally identical to the result I was getting before.

Since then I've tried to make several changes to the function to fix it, and, very strangely, I'm getting an identical wrong result every time. (I'm sure because I name the output file something different on every run.)

In the output I'm getting, many of the letters are now correct, many are wrong, and several are the numbers that correspond to the letters they should be (eg: 'T' keeps printing as 84). It also looks like it's still affecting numbers, spaces, and punctuation.

Could this be a problem with my computer? I'm really at a loss here...
Thanks again for your help.

[kethgr]

It was a problem with my computer...or probably compiler.

I copied all my code into a new project, and now it works great!

Thanks for your help everyone!

[cire]

Occasionally things get borked up somewhere in the tool chain and your compiler won't recognize that a source file has been updated, especially if you have some sort of incremental compiling enabled. Doing a full rebuild (or a clean and build) will usually fix the problem.

[kethgr]

Good advice. Thank you.