<go*************@hotmail.comschrieb im Newsbeitrag
news:11**********************@m73g2000cwd.googlegr oups.com...
Hi,
Im new to C++ and trying to self teach myself whilst I sit at my
unentertaining day job (thought i'd put that across before im accused
of cheating on my homework, im 45...)
Anyway I'm trying to dynamically assign a structure whilst I read from
a file, however my program crashes, and im not sure why other than that
its to do with my memory operations using new and delete.
Im pretty good at C so the reason for learning is so that I can truely
put C/C++ on my CV instead of just C... but also im interested in the
object orrientated aspect.
Could I use a class instead of a structure? whats best? where am I
going wrong?
Thanks
Heres my Code -
#include <iostream>
#include <fstream>
#include <string>
using namespace std;
struct TOKEN
Don't use identifiers with upper-case letters only. Those names should
better be reserved for identifiers used for #define'd macros.
{
string linetoken;
};
int main()
{
TOKEN *Tokens;
You should always initialize your variables, especially pointers.
int counter = 0;
Why do you write Tokens with an upper-case T but counter with a lower-case
c? Try to find a consistent naming convention. It makes life a lot easier if
you don't have to remember which name to start with an upper-case letter
andwhich one with a lower-case letter.
ifstream ifs("data.txt");
string line;
while(getline(ifs,line))
{
Tokens = new TOKEN[counter + 1];
This creates an array of counter+1 TOKENs and assigns the address of the
first element in the array to Tokens. Initially it allocates an array with
just one element and assigns its address to Tokens, overwritting Tokens's
undefined value. Nothing wrong so far.
On the next run, it allocates a NEW array with 2 elements and assigns the
address of the first element to Tokens, overwriting the old value of Tokens,
the array allocated during the previous pass through the loop. That results
in a memory leak, one in every round through the loop, except for the first
one.
Tokens[counter].linetoken = line;
This stores the contents of line in the last element of the newly allocated
array. All other elements of the array will contain default values, empty
strings in this case.
//cout << "[ " << line << " ]" << endl;
cout << Tokens[counter].linetoken << endl;
counter = counter + 1;
}
cout << "AAAAAAAA";
delete Tokens;
This causes undefined behaviour. You must only delete something you got from
new. If you used new[] to allocate memory, you have to use delete[] to free
it again.
return 0;
}
You have to allocate one array large enough to hold all the lines in
advance. Just because you allocate larger ones in each loop and assign their
address to the same variable, does not copy the contents of the previous
array to the new one. If you find out, that your array isn't large enough,
you have to create a new, larger one, copy all existing data from the old
array to the new one and finally delete[] the old array before you overwrite
its address with that of the new array.
Or you better learn about containes like std::list or std::vector. They are
much easier to use then C style arrays allocated with new[]/delete[].
Compare this with your code:
int main()
{
std::vector<TOKENtokens;
std::ifstream ifs("data.txt");
std::string line;
while (std::getline(ifs, line))
{
tokens.push_back(line);
std::cout << tokens.back().linetoken << std::endl;
}
std::cout << tokens.size() << " lines read" << std::endl;
}
HTH
Heinz
