«

Hi All,

I am a c/perl programmer trying my hand at C++.
In my code below I have an array class where I am trying to add two
arrays using "+" .
I am not sure why I get a '0' always for the first element of the result

My code ( slightly longish , I hope you would bear with a newbie )
--------------------------
using namespace std;
#include <iostream>
#define MAXARRAYSIZE 500

class MyArr {
public:
int *data,length;

MyArr(){
data = new int[0];
length = 0;
}

~MyArr(){
if(length) free(data);
}

void newFill(int *d,int l,bool freeD = true){
length = l;
if(freeD) free(data);
data = (int*) malloc(sizeof(int)*(length +2));
for(int i=0;i<length;i++) data[i] = d[i];
}

void dispArr() {
for(int i=0;i<length;i++) cout << data[i] <<"\t";
cout << "\n";
}

MyArr operator + ( const MyArr &r){
int a[length];
MyArr ret;
for(int i=0;i<length;i++) a[i]= data[i] + (r.data)[i];
ret.newFill(a,length);
return(ret);
}

};

int main(){
MyArr a1,a2;

{
int d[]={0,1,2,3,4};
int d2[]={11,12,13,14,15};
a1.newFill(d,5);
a2.newFill(d2,5);

}

a1.dispArr();
a2.dispArr();

a2 = a1 + a2;
a2.dispArr();
return(0);
}
---------------------- END -------------------

Here is the output

0 1 2 3 4
11 12 13 14 15

0 13 15 17 19
------> I expect the first element of last row = 11




Thanks
Ram

Ramprasad A Padmanabhan wrote:

You should consider valarray.

>
> class MyArr {
> public:
> int *data,length;
You should at least consider using vector here.
>
> MyArr(){
> data = new int[0];
> length = 0;
> }
>
> ~MyArr(){
> if(length) free(data);
You leak memory here. Even when you allocate int[0], it does return
something that needs to be deleted.

Further, your program had undeifned behavior. If you allocate via new[]
you must delete with delete [], not FREE.
delete [] data.


Further, you do all this managment, but you are incompelte. You need to handle
the case of copy construction and copy assignment as well, for the compiler generated
defaults for these functions will do the wrong thing.

Again, if you use vector<int> rather than mismanaging pointers, all this will be
handled for you by already debugged code.
> }

>
> void newFill(int *d,int l,bool freeD = true){
> length = l;
> if(freeD) free(data);
What is the point of this? If you don't free the data, it will be lost.
> data = (int*) malloc(sizeof(int)*(length +2));
What's with the slop here, and why are you switching ot malloc now?
> for(int i=0;i<length;i++) data[i] = d[i];
> }
>
> void dispArr() {
> for(int i=0;i<length;i++) cout << data[i] <<"\t";
> cout << "\n";
> }
>
> MyArr operator + ( const MyArr &r){

Congratulations on thinking about const finally, but all the functions
that don't modify MyArr should also be declared const.

> int a[length];

Arrays in C++ can not be declared with variable length.
Why not just use the internal array inside ret?

> MyArr ret;
> for(int i=0;i<length;i++) a[i]= data[i] + (r.data)[i];
> ret.newFill(a,length);
> return(ret);

This returns a copy and will invoke the bogus (compiler generated) copy constructor.

Ron Natalie wrote:
> Ramprasad A Padmanabhan wrote:
>
> You should consider valarray.
>
>>
>> class MyArr {
>> public:
>> int *data,length;
>
> You should at least consider using vector here.
>
>>
>> MyArr(){
>> data = new int[0];
>> length = 0;
>> }
>>
>> ~MyArr(){
>> if(length) free(data);
>
> You leak memory here. Even when you allocate int[0], it does return
> something that needs to be deleted.
>
> Further, your program had undeifned behavior. If you allocate via new[]
> you must delete with delete [], not FREE.
> delete [] data.
>
>
> Further, you do all this managment, but you are incompelte. You need
> to handle
> the case of copy construction and copy assignment as well, for the
> compiler generated
> defaults for these functions will do the wrong thing.
>
> Again, if you use vector<int> rather than mismanaging pointers, all this
> will be
> handled for you by already debugged code.
>
>> }
>
>
>>
>> void newFill(int *d,int l,bool freeD = true){
>> length = l;
>> if(freeD) free(data);
>
> What is the point of this? If you don't free the data, it will be lost.
>
>> data = (int*) malloc(sizeof(int)*(length +2));
>
> What's with the slop here, and why are you switching ot malloc now?
>
>> for(int i=0;i<length;i++) data[i] = d[i];
>> }
>>
>> void dispArr() {
>> for(int i=0;i<length;i++) cout << data[i] <<"\t";
>> cout << "\n";
>> }
>>
>> MyArr operator + ( const MyArr &r){
>
>
> Congratulations on thinking about const finally, but all the functions
> that don't modify MyArr should also be declared const.
>
>> int a[length];
>
>
> Arrays in C++ can not be declared with variable length.
> Why not just use the internal array inside ret?
>
>> MyArr ret;
>> for(int i=0;i<length;i++) a[i]= data[i] + (r.data)[i];
>> ret.newFill(a,length);
>> return(ret);
>
>
> This returns a copy and will invoke the bogus (compiler generated) copy
> constructor.


Ok I agree the code looks messy ( but hey I am just learning C++)
I thought I should bother about memory leak later , So I dropped my
destructor.
Now my code is working fine. So how do I free the memory allocated to
int *data

Thanks
Ram

"Ramprasad A Padmanabhan" <> wrote in
message news:Vq5Kd.32$...
> In my code below I have an array class where I am trying to add two arrays
> using "+" .
> I am not sure why I get a '0' always for the first element of the result
>
> My code ( slightly longish , I hope you would bear with a newbie )
Hi, allow me to make some comments along the way...
> --------------------------
> using namespace std;
> #include <iostream>
NB: the 2 previous lines should be swapped to compile without error
> #define MAXARRAYSIZE 500
In C++, we like to use enum { x = 500 }; or const int x = 500;
instead of macros, as these alternatives will respect C++ scopes.

> class MyArr {
> public:
> int *data,length;
(Many will prefer two separate declarations for clarity.)

> MyArr(){
> data = new int[0];
> length = 0;
While it is legal to allocate a 0-size array, why not
use a NULL pointer value instead?
> }
>
> ~MyArr(){
> if(length) free(data);
1) Based on the constructor, you will leak memory if length=0.
But it will be ok if you initialize data with NULL instead
(and you can omit the if(length) test anyway).
2) Memory allocated with new[] must be freed with delete[] :
delete[] data;

> }
>
> void newFill(int *d,int l,bool freeD = true){
> length = l;
> if(freeD) free(data);
> data = (int*) malloc(sizeof(int)*(length +2));
1) be consistent: use new int[length] here as well.
2) For exception/error safety, it is usually a good
idea to allocate the new memory before freeing
the previous data.
3) Consider using 'const': because the array pointed
to by d is not modified by the function, best
would be to declare the parameter as: int const* d

> for(int i=0;i<length;i++) data[i] = d[i];
> }
>
> void dispArr() {
> for(int i=0;i<length;i++) cout << data[i] <<"\t";
> cout << "\n";
> }
>
> MyArr operator + ( const MyArr &r){
> int a[length];
This array of a non-const dimention is not legal in ISO C++ 98
(although its has become legal in ISO C'99, and probably will
at some point be included in standard C++ as well).
Why not first allocate the memory for 'ret', and then
compute its new contents in place ?

> MyArr ret;
You probably need to check that (*this) and (r) have
compatible lengths !
> for(int i=0;i<length;i++) a[i]= data[i] + (r.data)[i];
> ret.newFill(a,length);
> return(ret);
> }

The problem with your class is that it lacks a copy-constructor
(and an assignment operator):
MyArr( MyArr const& orig );
MyArr& operator=( MyArr const& orig );
This is what causes the error
you observe in your test code.
> };

Rather than fixing the errors in your class, the best would
really be to use std::vector instead of a naked array.
This will allow the compiler to automatically and correctly
generate the additional member functions you need:

class MyArr {
public:
std::vector<int> data;

MyArr() : data () {}
MyArr(int count) : data(count) {}

// the default-generated destructor and copy-ctr/op are ok


// same as before, uses data.size() instead of length :
void dispArr() {
for(int i=0;i<data.size();i++) cout << data[i] <<"\t";
cout << "\n";
}

MyArr operator + ( const MyArr &r)
{
assert( r.data.size() == this->data.size() );
MyArr ret(this.length);
for(int i=0;i<data.size();++i)
ret.data[i] = this->data[i] + r.data[i];
return ret;
}
};


Finally, the class you are writing looks very much like
std::valaray<int>, available in the standard C++ library.
Alternatively, you may also want to consider using
a template parameter to specify the size of the array
(and even the element type)... but that's another story.


I hope this helps,
Ivan
--
http://ivan.vecerina.com/contact/?subject=NG_POST <- email contact form
Brainbench MVP for C++ <> http://www.brainbench.com

"Ramprasad A Padmanabhan" <> wrote in
message news:...
> I thought I should bother about memory leak later , So I dropped my
> destructor.
> Now my code is working fine. So how do I free the memory allocated to int
> *data
As Ron and I pointed out, the problem is not (just) the destructor,
but the copy-constructor and copy-assignment operator that you need
to implement as well.
But you don't need to bother, just use std::vector ( or std::valarray ).


--
http://ivan.vecerina.com/contact/?subject=NG_POST <- email contact form

......
>
>
> Finally, the class you are writing looks very much like
> std::valaray<int>, available in the standard C++ library.
> Alternatively, you may also want to consider using
> a template parameter to specify the size of the array
> (and even the element type)... but that's another story.
>
>
> I hope this helps,
> Ivan

That sure helps Ivan, thanks a lot.
I learnt I have a long way to go still. I have not yet got into vectors
and std::*
But I would consider trying out something what I understand , I mean
using a *naked array*, first and write some code. I have always learnt
laguages best by coding. I sure will try what you suggested once I get a
hang of it all.

BTW if I drop my destructor the code works fine, any idea why ?

Thanks
Ram

Ramprasad A Padmanabhan wrote:
>
> .....
> >
> >
> > Finally, the class you are writing looks very much like
> > std::valaray<int>, available in the standard C++ library.
> > Alternatively, you may also want to consider using
> > a template parameter to specify the size of the array
> > (and even the element type)... but that's another story.
> >
> >
> > I hope this helps,
> > Ivan
>
> That sure helps Ivan, thanks a lot.
> I learnt I have a long way to go still. I have not yet got into vectors
> and std::*
> But I would consider trying out something what I understand , I mean
> using a *naked array*, first and write some code. I have always learnt
> laguages best by coding. I sure will try what you suggested once I get a
> hang of it all.

The point is that you *first* should learn to use std::vector
(and std::string and std::list and all the other container available)
*before* you put your hands onto naked dynamically allocated arrays.

You should start to write programs without having to deal with this
low level stuff. Once you get fluent in programming you can always
come back and figure out how all if them work under the hood by studying
implementation of 'naked dynamically allocated' data structures.

You don't learn maching by studying how atoms are bonded to form molecules.
But this is what you are doing right now.

>
> BTW if I drop my destructor the code works fine, any idea why ?

Because you hide the error by not cleaning up the memory.
Your code still doesn't work fine (it is leaking memory), but
you don't notice, because there is no visible indication of it.

Remember: You never can prove the absence of errors by testing.
You can only prove the presence of errors.

--
Karl Heinz Buchegger

"Ramprasad A Padmanabhan" <> wrote in
message news:Vq5Kd.32$...
> Hi All,
>
> I am a c/perl programmer trying my hand at C++.
> In my code below I have an array class where I am trying to add two
> arrays using "+" .
> I am not sure why I get a '0' always for the first element of the result
>
> My code ( slightly longish , I hope you would bear with a newbie )
> --------------------------
> using namespace std;
> #include <iostream>
> #define MAXARRAYSIZE 500
>
> class MyArr {
> public:
> int *data,length;
>
> MyArr(){
> data = new int[0];
> length = 0;
> }
>
> ~MyArr(){
> if(length) free(data);
> }
>
> void newFill(int *d,int l,bool freeD = true){
> length = l;
> if(freeD) free(data);
> data = (int*) malloc(sizeof(int)*(length +2));
> for(int i=0;i<length;i++) data[i] = d[i];
> }
>
> void dispArr() {
> for(int i=0;i<length;i++) cout << data[i] <<"\t";
> cout << "\n";
> }
>
> MyArr operator + ( const MyArr &r){
> int a[length];
> MyArr ret;
> for(int i=0;i<length;i++) a[i]= data[i] + (r.data)[i];
> ret.newFill(a,length);
> return(ret);
> }
>
> };
>
> int main(){
> MyArr a1,a2;
>
> {
> int d[]={0,1,2,3,4};
> int d2[]={11,12,13,14,15};
> a1.newFill(d,5);
> a2.newFill(d2,5);
>
> }
>
> a1.dispArr();
> a2.dispArr();
>
> a2 = a1 + a2;
> a2.dispArr();
> return(0);
> }
> ---------------------- END -------------------
>
> Here is the output
>
> 0 1 2 3 4
> 11 12 13 14 15
>
> 0 13 15 17 19
> ------> I expect the first element of last row = 11

Among the many, many things wrong with this code (which I leave to more
patient respondents to point out ), what's basically causing your
erroneous output is that your MyArr class lacks an assignment operator:

MyArr& operator = (const MyArr& a);

Thus in the line:

a2 = a1 + a2;

in main(), since no assignment operator is defined, the default
element-by-element copy will be used. This will simply copy the *pointer*
"data" member of the *temporary* MyArr object "ret" returned by operator +
to the "data" member of a2, without also copying across the contents of the
memory pointed to by "ret.data" - which is what you want. Thus - once the
temporary "ret" goes out of scope - "a2.data" is left pointing to an
undefined area of memory, leading to undefined behaviour; your program could
have done anything (including aborting or defrosting your fridge).

You might want to look up the so-called "Law of the Big Three":
http://www.parashift.com/c++-faq-lit....html#faq-27.9

Regards,

--
Lionel B

Karl Heinz Buchegger wrote:
> Ramprasad A Padmanabhan wrote:
>
>>.....
>>
>>>
>>>Finally, the class you are writing looks very much like
>>>std::valaray<int>, available in the standard C++ library.
>>>Alternatively, you may also want to consider using
>>>a template parameter to specify the size of the array
>>>(and even the element type)... but that's another story.
>>>
>>>
>>>I hope this helps,
>>>Ivan
>>
>>That sure helps Ivan, thanks a lot.
>>I learnt I have a long way to go still. I have not yet got into vectors
>>and std::*
>>But I would consider trying out something what I understand , I mean
>>using a *naked array*, first and write some code. I have always learnt
>>laguages best by coding. I sure will try what you suggested once I get a
>>hang of it all.
>
>
> The point is that you *first* should learn to use std::vector
> (and std::string and std::list and all the other container available)
> *before* you put your hands onto naked dynamically allocated arrays.
>
> You should start to write programs without having to deal with this
> low level stuff. Once you get fluent in programming you can always
> come back and figure out how all if them work under the hood by studying
> implementation of 'naked dynamically allocated' data structures.
>
> You don't learn maching by studying how atoms are bonded to form molecules.
> But this is what you are doing right now.
>
>
>>BTW if I drop my destructor the code works fine, any idea why ?
>
>
> Because you hide the error by not cleaning up the memory.
> Your code still doesn't work fine (it is leaking memory), but
> you don't notice, because there is no visible indication of it.
>
> Remember: You never can prove the absence of errors by testing.
> You can only prove the presence of errors.
>

OK OK. I get it now . First thing I will do know will be to learn all
the std::* ( which seemed too much of jargon for a beginner all this
while ).
I hope you see the point, I am trying to learn by myself and since
I am going by the order of the chapters in a online resource I have not
yet reached there.
Do you have any suggestions, Any online resource I could use.

Thanks
Ram

"Ramprasad A Padmanabhan" <> wrote in
message news:...
Hi,
> OK OK. I get it now . First thing I will do know will be to learn all
> the std::* ( which seemed too much of jargon for a beginner all this
> while ).
> I hope you see the point, I am trying to learn by myself and since I
> am going by the order of the chapters in a online resource I have not yet
> reached there.
Yes, but coming from a C background, it is probably a good idea to
focus on the high-level aspects of C++ first.

> Do you have any suggestions, Any online resource I could use.
The best books (sorry) to start with are probably "Accelerated C++"
(as it starts from the non-C end of C++) -- see www.acceleratedcpp.com
and the two "(More) Effective C++" books (or the combined CD version).
For the latter, take a look at the item titles available at:
http://www.awprofessional.com/conten...DEMO/INDEX.HTM
Stroustrup's "The C++ Programming Language", 3rd or 'special' edition,
has a very good coverage of the language overall.
Best IMHO for the library itself, Josuttis' C++ std lib tutorial & ref:
http://www.josuttis.de/libbook/index.html (just an overview here).

Then there is the more advanced and specialized...

Online:

The "Thinking in C++" books are available in a free electronic form,
they are not bad.
See: http://www.mindview.net/Books/TICPP/...ngInCPP2e.html

Studying the GOTW items (which are the basis of Herb Sutter's
excellent Exceptional C++ books) can also be very instructive.
The books have corrections and an improved text/organization, but the
raw materials are available at: http://www.gotw.ca/gotw/index.htm

The standard library (including some vendor-specific extensions)
is rather well documented on SGI's STL website:
http://www.sgi.com/tech/stl/table_of_contents.html
Also very good, but with impaired browsing, dinkumware's online
reference: http://www.dinkumware.com/manuals/reader.aspx?lib=cpp


Of course this is not a complete list, just my first thoughts...

Cheers,
Ivan
--
http://ivan.vecerina.com/contact/?subject=NG_POST <- email contact form
Brainbench MVP for C++ <> http://www.brainbench.com