Thread: Searching a text file backwards

Good to hear, but I have a few suggestions to make:

• Break up your main function into smaller functions that do one thing and do it well.
• #include <vector> and change buffer to a std::vector<char> into of doing manual memory management.
• Declare variables near first use. If possible, declare them at the point where you can provide them with sensible initial values. In fact, you should change this:
  
  Code:

  ifstream inFile;	// Initiate the input file stream
  inFile.open("C:\\datafile.txt", ifstream::in);

  to a one-liner that initialises the object:
  
  Code:

  ifstream inFile("C:\\datafile.txt");

• You only use the non-standard getch() at the end, presumably to pause the program. Since you are not using anything else from the non-standard <conio.h>, you might as well get rid of that header inclusion and use a more portable alternative to getch() for this purpose.

Your solution of overlapping buffers is a good one. I notice you're never really concerned with using the image number as a number; you just care about it matching. You could change your search string to hold the image number as well. That would get rid of a chunk of awkward code.

Also, you must initialize imgNum to a value. Right now it contains a garbage value when it is compared against desiredImageNumber for the first time. You don't really need to new/delete your buffer of memory every time through the loop; that causes unnecessary memory thrashing of the heap. You should handle that outside of your loop.

Code:

	inFile.seekg (0, ios::end);	
	eofPtrPosn = inFile.tellg();	
	eofPtrPosn = eofPtrPosn; /* This line does nothing; perhaps you meant currentPtrPosn? */
	currentPtrPosn = inFile.tellg(); // IF ONLY I KNEW HOW TO DO THINGS INITIALLY THEN DO A LOOP
	while (desiredImageNumber != imgNum){

You can use a for-loop if you prefer to have an initialization part to your loop:

Code:

for(eofPtrPosn = inFile.tellg(), currentPtrPosn = eofPtrPosn; desiredImageNumber != imgNum; ) {

Originally Posted by Geek10

Originally Posted by laserlight

#include <vector> and change buffer to a std::vector<char> into of doing manual memory management.

Sorry, I don't quite understand this sentence. Could you rephrase it maybe?

The more you learn, the more you realize that having new's and delete's scattered through your code is a bad idea. Fortunately, a lot of standard classes handle this sort of dynamic memory for you.

laserlight is pointing out that you can use a std::vector<char> instead of your current char* buffer. The main benefit here is memory safety. Right now if an exception occurred in your while loop or you changed your code to break out of the loop before the delete, you could end up with a memory leak because delete isn't called. Using a std::vector, the memory is guaranteed to be cleaned up once the vector leaves scope.

Code:

	while (desiredImageNumber != imgNum){
		std::vector<char> buffer(bufferSize); 		// to store search search block of code
		// ...
		// read data into buffer
		inFile.read (&buffer[0],bufferSize);
		// ...
	}

Notice that no new or delete is necessary. I left the vector inside the loop so that you could see a direct comparison with your code, but it should really be declared outside the loop so that it is only created once.

Originally Posted by Geek10

Good point. The only problem is I haven't yet found an alternative. I'll try and have a look around when I think of it. For now, getch() works to pause the program for me to see the command prompt!

Believe it or not, Cprogramming.com has an FAQ section. It even has a main website. (For a long time, I thought the forums were the only thing here.) One of the FAQs: How do I get my program to wait for a keypress?