Dynamic Structures

[googlinggoogler@hotmail.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
{
string linetoken;
};

int main()
{

TOKEN *Tokens;

int counter = 0;
ifstream ifs("data.txt") ;
string line;

while(getline(i fs,line))
{

Tokens = new TOKEN[counter + 1];

Tokens[counter].linetoken = line;

//cout << "[ " << line << " ]" << endl;
cout << Tokens[counter].linetoken << endl;
counter = counter + 1;
}

cout << "AAAAAAAA";

delete Tokens;
return 0;
}

[Philip Potter]

Re: Dynamic Structures

<googlinggoogle r@hotmail.comwr ote in message
news:1156847343 .243259.246770@ m73g2000cwd.goo glegroups.com.. .

Anyway I'm trying to dynamically assign a structure whilst I read from
a file, however my program crashes,

How far does it get before crashing? Does it display "AAAAAA" first?

and im not sure why other than that
its to do with my memory operations using new and delete.
>
Could I use a class instead of a structure? whats best? where am I
going wrong?

Anything you can do with a class you can do with a structure, and vice
versa. The difference is the default visibility (private for class, public
for struct).

#include <iostream>
#include <fstream>
#include <string>
>
using namespace std;
>
struct TOKEN

I wouldn't have made that uppercase. (But your style != my style.)

{
string linetoken;
};
>
int main()
{
>
TOKEN *Tokens;
>
int counter = 0;
ifstream ifs("data.txt") ;
string line;
>
while(getline(i fs,line))
{
>
Tokens = new TOKEN[counter + 1];

new[] is analogous to malloc(), not to realloc(). You're leaking memory each
time through this loop beyond the first, because you lose the memory Tokens
used to point at.

This is why in C++ container classes are usually used instead. It'd be much
easier to use a std::vector<TOK ENhere, then you could just do
token.push_back (TOKEN(counter + 1)); each time round the loop and not have
to worry about memory allocation.

On the other hand, if you are doing this as an excercise for yourself in
dynamic memory management, perhaps you should create your own container
class. Call it something like TokenContainer, TokenList or TokenArray and
provide a method to add a new token on the end, and another to get values
from it. You can implement it as a resizable array, or as a linked list, or
in any other way you like. Focus on correctness over speed. Make sure your
destructor deletes everything in the container cleanly. Use a memory leak
detector (such as valgrind) to check if you really did delete everything.

Once you have created your TokenContainer and got it working, throw it away
and use std::vector, std::deque or std::list. They're likely to be much more
stable and much faster than your container :)

Tokens[counter].linetoken = line;
>
//cout << "[ " << line << " ]" << endl;
cout << Tokens[counter].linetoken << endl;
counter = counter + 1;
}
>
cout << "AAAAAAAA";
>
delete Tokens;

Wrong operator. This should be:
delete[] Tokens;
since you used new to create an array instead of a single object. This is
possibly the cause of the crash.

return 0;
}
>

[Heinz Ozwirk]

Re: Dynamic Structures

<googlinggoogle r@hotmail.comsc hrieb im Newsbeitrag
news:1156847343 .243259.246770@ m73g2000cwd.goo glegroups.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(i fs,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<TOK ENtokens;
std::ifstream ifs("data.txt") ;
std::string line;
while (std::getline(i fs, line))
{
tokens.push_bac k(line);
std::cout << tokens.back().l inetoken << std::endl;
}
std::cout << tokens.size() << " lines read" << std::endl;
}

HTH
Heinz

[Volker Lukas]

Re: Dynamic Structures

googlinggoogler @hotmail.com wrote:

Heres my Code -
>
#include <iostream>
#include <fstream>
#include <string>

I recommend to also include <ostream>, because you use "endl" later in your
code. Including <iostreamand/or <fstreamdoes not guarantee to make endl
available. Or, instead of using endl to display a newline you could just
output '\n', if you do not need to flush the output at the same time.

[Jim Langston]

Re: Dynamic Structures


<googlinggoogle r@hotmail.comwr ote in message
news:1156847343 .243259.246770@ m73g2000cwd.goo glegroups.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
{
string linetoken;
};
>
int main()
{
>
TOKEN *Tokens;
>
int counter = 0;
ifstream ifs("data.txt") ;
string line;
>
while(getline(i fs,line))
{
>
Tokens = new TOKEN[counter + 1];
>
Tokens[counter].linetoken = line;
>
//cout << "[ " << line << " ]" << endl;
cout << Tokens[counter].linetoken << endl;
counter = counter + 1;
}
>
cout << "AAAAAAAA";
>
delete Tokens;
return 0;
}

This is not the way I would design this. But...

You have a TOKEN pointer. I presume at the end you want to have this Token
pointer contain a dynamic array of all the tokens (lines in the file).

What you are doing:

while ( data exists )
read line.
Allocate array of tokens with number of lines so far
assign last element to current line
Delete array.

The problem with this is you loose the previous data in a memory leak.
There are much better algorithms, but you could make yours work if you did
this:

while ( data exists )
read line
allocate array of tokens with number of lines so far
copy all the old strings into the array
delete old array
assign last telement to current line
delete array

Note: The following is untested code, but should give you the idea.

#include <iostream>
#include <fstream>
#include <string>
>
using namespace std;
>
struct TOKEN
{
string linetoken;
};
>
int main()
{
>
TOKEN *Tokens;
>
int counter = 0;
ifstream ifs("data.txt") ;
string line;
>
while(getline(i fs,line))
{
>

// Save pointer to aready allocated tokens
TOKEN* LastTokens = Tokens;

Tokens = new TOKEN[counter + 1];

// Note, this only works if counter == 0 because even though Tokens hasn't
been allocated
// yet, we actually don't do anything for an empty array because 0 < 0 evals
false on
// first run though.
// std::copy could also be used I believe
for ( int i = 0; i < counter; ++i )
Tokens[i] = LastTokens[i];
delete[] LastTokens;

>
Tokens[counter].linetoken = line;
>
//cout << "[ " << line << " ]" << endl;
cout << Tokens[counter].linetoken << endl;
counter = counter + 1;
}
>
cout << "AAAAAAAA";

// At this point, Tokens points to a dynamically allocated array of Tokens
containing all the lines in the file.

>
delete Tokens;

change this to
delete Tokens[];

return 0;
}

This is a rather ineffecient way to do this, because for n elements you have
n memory allocations and !n copies. A container such as vector does
something similar but it will allocate the previous allocated space *2 each
time so memory allocations and copies are only done when memory allocation
takes place. Of course this means the container has to keep track of how
much memory is allocated for how many elements and how many elements
actually have data in them.

[Ron Natalie]

Re: Dynamic Structures

Heinz Ozwirk wrote:

>struct TOKEN

>
Don't use identifiers with upper-case letters only. Those names should
better be reserved for identifiers used for #define'd macros.

Oh posh. This is C++, not decade old C. Macros are rarely used.

>

[Frederick Gotham]

Re: Dynamic Structures

Ron Natalie posted:

>>struct TOKEN

>>
>Don't use identifiers with upper-case letters only. Those names should
>better be reserved for identifiers used for #define'd macros.

>
Oh posh. This is C++, not decade old C. Macros are rarely used.

Heinz gave very good advice -- reserve the use of ALL-UPPERCASE for macro
names only (regardless of how frequently or rarely you use them).

--

Frederick Gotham

[Jens Theisen]

Re: Dynamic Structures

Frederick Gotham wrote:

Heinz gave very good advice -- reserve the use of ALL-UPPERCASE for macro
names only (regardless of how frequently or rarely you use them).

Let's hope he was joking.

Jens

[Victor Bazarov]

Re: Dynamic Structures

Jens Theisen wrote:

Frederick Gotham wrote:

>Heinz gave very good advice -- reserve the use of ALL-UPPERCASE for
>macro names only (regardless of how frequently or rarely you use
>them).

>
Let's hope he was joking.

Why? It's a good style advice. Makes macros visible. They have their
special traits and often need to be easily identifialbe throughout the
code.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[Jens Theisen]

Re: Dynamic Structures

Victor Bazarov wrote:

Why? It's a good style advice. Makes macros visible. They have their
special traits and often need to be easily identifialbe throughout the
code.

Sorry if it wasn't clear, but I was referring to

Oh posh. This is C++, not decade old C. Macros are rarely used

which probably was a joke.

Jens

[Ron Natalie]

Re: Dynamic Structures

Victor Bazarov wrote:

Jens Theisen wrote:

>Frederick Gotham wrote:

>>Heinz gave very good advice -- reserve the use of ALL-UPPERCASE for
>>macro names only (regardless of how frequently or rarely you use
>>them).

>Let's hope he was joking.

>
Why? It's a good style advice. Makes macros visible. They have their
special traits and often need to be easily identifialbe throughout the
code.

It's antiquated style advice and has never been held to even by the
language/libraries itself (FILE isn't a macro, and assert is, for
example).

A better advice would be to avoid preprocessor macros if possible
in C++. If you do that, there's no point in wasting a whole class
of variable names on something you never do.

[Victor Bazarov]

Re: Dynamic Structures

Ron Natalie wrote:

Victor Bazarov wrote:

>Jens Theisen wrote:

>>Frederick Gotham wrote:
>>>Heinz gave very good advice -- reserve the use of ALL-UPPERCASE for
>>>macro names only (regardless of how frequently or rarely you use
>>>them).
>>Let's hope he was joking.

>>
>Why? It's a good style advice. Makes macros visible. They have
>their special traits and often need to be easily identifialbe
>throughout the code.

>
It's antiquated style advice and has never been held to even by the
language/libraries itself (FILE isn't a macro, and assert is, for
example).

Yes, but you know them, I know them, many others know them, so it's not
such a big deal to have a couple of exceptions to the rule.

A better advice would be to avoid preprocessor macros if possible
in C++. If you do that, there's no point in wasting a whole class
of variable names on something you never do.

Yes, as an orthogonal rule, the macros should be avoided. If/when they
cannot be, making their names uppercase is preferred.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[Victor Bazarov]

Re: Dynamic Structures

Jens Theisen wrote:

Victor Bazarov wrote:

>Why? It's a good style advice. Makes macros visible. They have
>their special traits and often need to be easily identifialbe
>throughout the code.

>
Sorry if it wasn't clear, but I was referring to
>

>Oh posh. This is C++, not decade old C. Macros are rarely used

>
which probably was a joke.

Ah...