Where to allocate memory for the char * data member ...

[marora]

I have created class definition which contains
a charater pointer as one of it's data memeber.

The objective is to read some data from a file,
and assign it to a data member;

Size of data is not known in the begining.
We can assume that it will not exceed 256;

class definition:

class abc {
private:
char * temp;
short int size;
public:

abc() {
size = 0;
temp = new char [size + 1];
};

~abc() {
delete temp;
};

friend istream& operator >>(istream &is, abc & obj);
char * Get_temp() {
return temp;
};
};

istream& operator >> (istream & is, abc &obj)
{
is >> obj.temp;
return is;
}

int main ()
{
abc object_abc;

ifstream infile("xyz.txt");
assert(infile);
assert(infile.is_open());

while (!infile.eof()) {
infile >> object_abc;
cout << object_abc.Get_temp() << endl;
}

infile.close();

return 0;
}

My question is how, when, and where to allocate memory for the data
member "temp".

 

[Luke Meyers]

My question is how, when, and where to allocate memory for the data
member "temp".

First of all, in your destructor, use delete[] rather than delete.
Otherwise you get a memory leak.

Now, on to your question. You've got a bit of a problem here, in that
your use of a naked array, with no bounds checking, is very much
unsafe. However big you make the array, if your input file is one byte
longer, you will get undefined behavior.

If you *really* want to accept an istream into your array, you should
look into stream manipulators. You could adopt a strategy whereby the
stream manipulator allocates more memory just before you need it.

Still, I'd avoid that altogether and just use a std::stringstream or
something. There's basically no good reason to use char* instead of
std::string, generally. And exposing a pointer to your internal
representation, especially a pointer to non-const, is just asking for
trouble (and no, a pointer to const isn't sufficient for safety -- they
can still delete it, for one thing).

Defensive programming is your friend.

Oh, and if you're absolutely sure about that 256 byte limit, and you
aren't making tons of these things, maybe just statically allocate an
array of size 256, rather than worrying about doing it dynamically.
But again... by the time you're doing that, you're already a good ways
down the path to perdition.

Luke

 

[Ron Natalie]

I have created class definition which contains
a charater pointer as one of it's data memeber.

The problems you are having and more are avoided if you
use an already designed and debugged class rather than
char*. If the object is a string, use the std::string
class. If it's an array of char, use vector.

short int size;

Why short?

abc() {
size = 0;
temp = new char [size + 1];
};

Why are you allocating one additional byte? Why is size zero.

~abc() {
delete temp;

Wrong delete. If you new[], you must do
delete [] temp;


You also have a rule of three violation. Your object will blow
up if copied or assigned. You need to do a copy constructor and
a copy assignment operator. Again this wouldn't be a problem if
temp were a vector or string. These classes already have these defined.

ifstream infile("xyz.txt");
assert(infile);
assert(infile.is_open());

Assert is a lousy way of detecting errors. These two tests are
redundant as well.

while (!infile.eof()) {

This is NOT the way to check for end of file. The eof state is
only set AFTER a read fails.

while(infile >> object_abc) {

will do what you want (it will fail when there is an error or eof, you
can then check the eof flag if you need to differentiate).

My question is how, when, and where to allocate memory for the data
member "temp".

You haven't told us how you know what the data is that you are reading?
What is the nature of the file?

You can't use a >> char* without knowing some upper bound on the length.
This isn't a limitation with the string class though.

As a matter of fact, I think you can probably deep-six your entire class:

int main() {
ifstring infile("xyz.txt");
if(!infile) return EXIT_FAILURE;

std::string abc;
while(infile >> abc) {
cout << abc << "\n";
// if you really wanted a char* out of it, you can use abc.c_str()
}
cout << flush;
}

 

[Manish]

short int size;
1. Why short? <<--- I know the size will not be greator than 256

2a. Why are you allocating one additional byte? <<--- I think I should
have initialilzed the data member "temp" to NULL.

2b. Why is size zero. <<-- Just initializing this to a value

"You also have a rule of three violation. Your object will blow
up if copied or assigned. You need to do a copy constructor and
a copy assignment operator. Again this wouldn't be a problem if
temp were a vector or string. These classes already have these
defined."

Yeap, can't do without the copy and assignment constructor.

3. Yeap! delete [] temp; <--- missed this one.

"This is NOT the way to check for end of file. The eof state is
only set AFTER a read fails.

while(infile >> object_abc) {

will do what you want (it will fail when there is an error or eof, you
can then check the eof flag if you need to differentiate). "

.... hmm ... I have to make a note of this one.



++++++++++++++++++++++++++++++++++++++++++++++++++++++
I could have used string type for the data members.
However, I didn't want the overhead that comes with the string class.

A sample data file, would be as follows

xyz.txt
some_name1 value1
some_name2 value2
..
..

Having said that my class definition needs two different data members,
i didn't include that in my previous post.

class abc {
....
char * temp;
char * temp2;
....
};

Other functions will definitely require amendments, and will
incorporate the second data member.
istream& operator >> (istream & is, abc &obj)
{
is >> obj.temp >> obj.temp2;
return is;
}

I really do appreciate your suggestion, besides using another class
string or stringstream, if I stick with "char *"
I am still not sure how, when, and where to allocate memory for "temp
and temp2" data members.

Regards,
Manish

 

[Jim Langston]

class abc {

....
char * temp;
char * temp2;
....
};

Other functions will definitely require amendments, and will
incorporate the second data member.
istream& operator >> (istream & is, abc &obj)
{
is >> obj.temp >> obj.temp2;
return is;
}

I really do appreciate your suggestion, besides using another class
string or stringstream, if I stick with "char *"
I am still not sure how, when, and where to allocate memory for "temp
and temp2" data members.

Since you don't know the size you are reading before you read it, and since
you don't want to use std::string, there is only one logical place to
allocate the memory for temp and temp2, in the class constructor. Delete
them in the class destructor.

That fixes the When and Where, the how is simple and you already know how to
use new.