Reading files with an unkown amount of characters

Well howdy folks!
This is my first thread/reply at this forum, and just so you know how good/bad I am at this (mostly bad :p ) I can tell that I know all there is worth to know about HTML, CSS and JavaScript (I know this ain't special). I have started to code in C++ (some jump in huh), and have already read the book "SAMS Teach Yourself C++ in 21 days". Besides this book I've read a bit about this subjet on the net. I have some experience and have, among others, maid this program that seeks for x strings through all files in a submenu or disk. My compiler is Borland 6 (legal it is too) and I know my way around it (not in details though).
As a last comment, before telling what my problem is, "I'm from Denmark, and this I consider as being a perfectly good excuse for all that bad spelling. Though I promise I'll do better next time. I'm just in a hurry at the moment" (Sure!)

This function is a part of a class I've named TFileHandle, which purpose is to take care of files, using streams, without having to do much.
Since a file can have, in theory, endless characters, I've decided to make the code this way.

Plz read my code and comment in betweem, and tell me what I'm doing wrong here. :confused:

Code:

---

AnsiString TFileHandle::Load() const
{
  char *Text = new char;
  Text[0] = '\0';
  char *Temp;
  char Char;

  unsigned int Lenght;

  ifstream File(itsName.c_str());

  if (! File.fail())
  {
    while(! File.eof())
    {
      Char = File.get();

      if (! File.fail())
      {
        Temp = Text;
        Lenght = strlen(Text);

        Text = new char[Lenght + 1];

        strcpy(Text, Temp);
        delete[] Temp; /* <- If I remove this line the program works,
                          but then I've defiantly crated a memory leak :( */

        Text[Lenght] = Char;
        Text[Lenght + 1] = '\0';
      }
    }
  }

  File.close();

  return Text;
}

---

O sorry. I might wan a add that the reason why I'm not using AnsiString objectet for this is that I wan it to be fast (it have to read allot of files in not so much time). I know that I have used AnsiString but that's only because I at first tryed to make it work with AnsiString. Now I need it to work with char.

:p :p You are "newing" one char at a time and you want this to e fast. Id totally rethink your approach and try again. Each call to new will cost you in efficiency terms about 8* a normal function call. So "newing" a single char for everyone read would really slow this up like glue.

Umm where to begin.

First thing you are redeclaring Text.


//is
char *Text = new char;
Text = new char[Lenght + 1];

//Should be
char *Text;
Text = new char[Lenght + 1];

Second, you need to delete Text and not Temp. Temp is not using any memory. When you do Temp=Text; you are pointing your Temp pointer to the space in memory where Text is. strcpy(Text, Temp); does nothing in this case. So, Text and Temp both contain a memory address which is the same. Remove the delete [] Temp; If you need to keep Text around then delete it in your destructor. delete [] Text;

What I think you really want to do is this:

Temp = new char[Length + 1]; // Create temp with enough space
strcpy(Temp,Text); //Copy old text in
strcat(Temp,Char); // Add the new char read
delete [] Text; // Delete the old space used by Text
Text=new char[Length + 1]; // Create a new Text of the new size
strcpy(Text,Temp); // Copy Temp into Text
delete [] Temp; // Delete the Temp space

Sounds obvius when you put it like that. Though *Temp and *Text doesn't point at the same addres because of:
Text = new char[Lenght + 1];
This is why I used:
strcpy(Text, Temp);
Because I wanted the old text to be put into the renewed Text with one more char.

Besides I know that "newing" every one char demands allot of computer processing, but the reason I do this is that if you don't know how many characters there is in one line you can't just say getline() and define length as 1024 or something else. Because what if it turns out to be any longer.

Well anyway you guys got me going so... thx. :)

Here is another thought. You are going to end up with, breifly, with 2 arrays that are as large as your file. Maybe we can avoid this. I have not tried this, but...


char *Text;

if (! File.fail())
{
Text=new char(Char);
}

From what I have been told (and it could be wrong) this is acceptable. That what happens is that the new char is added onto the end of the the Text pointer. So Text becomes indexed, even though we haven't defined it that way. Which means when you delete it you would need to do a delete [] Text;. I have not tested this, but if it works, it makes your code much shorter and easier.

I've just tryed and it doesn't work. Would be to d*mn easy i gues. :D

I just tested it as well, it does not work. It sounded fishy when I heard it, bit it was in a c++ class. Can't beleive everything I guess.

>I might wan a add that the reason why I'm not using AnsiString objectet for this is that I wan it to be fast
Input and output is inherently slow to begin with, your choice of using a safer object such as std::string or std::vector or an error prone method of allocating your own char *'s is generally irrelevant when it comes to speed:

Code:

---

#include <string>

std::string TFileHandle::Load() const
{
  char ch;
  std::string input;

  std::ifstream file( itsName.c_str() );

  if ( file.is_open() ) {
    while ( file>>ch )
      input += ch;

    if ( !file.eof() )
      std::cerr<<"Error reading input file\n";

    file.close();
  }

  return input;
}

---

And on top of all this, you should never worry about speed until you see a signifigant need to improve performance.

-Prelude

Code:

---

        Lenght = strlen(Text) + strlen(&Char) + 1;

        Temp = new char[Lenght];

        strcpy(Temp, Text);
        strcat(Temp, &Char);
        delete [] Text;

        Text = new char[Lenght];
        strcpy(Text, Temp);
        delete [] Temp;

---

It still doesn't work, and I know for sure that this is the part of code which makes all the problems. :confused:

Prelude, still that doesn't explain why it doesn't work. I would really like to know.

>Prelude, still that doesn't explain why it doesn't work. I would really like to know.
Have you tried stepping through your code in a debugger? It's surprising how much you can learn about how your code is working when you do.

-Prelude

I think your problem is that you have declared Char to be a char, not a char * or a char[] which is what strlen() is expecting. Therefore strlen(&Char) won't work. Therefore Lenght is junk. Therefore memory allocation is junk. Yadda, yadda, yadda...

I would change Char to a string somehow. Either read in Char as you do and place it in the first index of a string like this:

//outside loops:
char dummy[2];
dummy[1] = '\0';


//inside loops;
Char = File.get();
dummy[0] = Char;

and then send dummy to strlen();

or change Char to be a string and use get() or getline() to read into it:

//outside loop
char Char[2];

//inside loop;
cin.get(Char, 1);

or

cin.getline(Char, 1);

Hey! That dummy thing hurts. :p
Well, thx elad, but since homework needs to get done I'll look into sometime tomorow. And yes I will complain if it still doesn't work. :D