Array object operator overloading

Discussion in 'C++' started by Ramprasad A Padmanabhan, Jan 27, 2005.

  1. 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 = d;
    }

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

    MyArr operator + ( const MyArr &r){
    int a[length];
    MyArr ret;
    for(int i=0;i<length;i++) a= data + (r.data);
    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, Jan 27, 2005
    #1
    1. Advertising

  2. Ramprasad A Padmanabhan

    Ron Natalie Guest

    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 = d;
    > }
    >
    > void dispArr() {
    > for(int i=0;i<length;i++) cout << data <<"\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= data + (r.data);
    > ret.newFill(a,length);
    > return(ret);


    This returns a copy and will invoke the bogus (compiler generated) copy constructor.
     
    Ron Natalie, Jan 27, 2005
    #2
    1. Advertising

  3. 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 = d;
    >> }
    >>
    >> void dispArr() {
    >> for(int i=0;i<length;i++) cout << data <<"\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= data + (r.data);
    >> 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, Jan 27, 2005
    #3
  4. "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 = d;
    > }
    >
    > void dispArr() {
    > for(int i=0;i<length;i++) cout << data <<"\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= data + (r.data);
    > 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 <<"\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 = this->data + r.data;
    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
     
    Ivan Vecerina, Jan 27, 2005
    #4
  5. "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
     
    Ivan Vecerina, Jan 27, 2005
    #5
  6. ......
    >
    >
    > 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, Jan 27, 2005
    #6
  7. 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
     
    Karl Heinz Buchegger, Jan 27, 2005
    #7
  8. Ramprasad A Padmanabhan

    Lionel B Guest

    "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 = d;
    > }
    >
    > void dispArr() {
    > for(int i=0;i<length;i++) cout << data <<"\t";
    > cout << "\n";
    > }
    >
    > MyArr operator + ( const MyArr &r){
    > int a[length];
    > MyArr ret;
    > for(int i=0;i<length;i++) a= data + (r.data);
    > 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-lite/coding-standards.html#faq-27.9

    Regards,

    --
    Lionel B
     
    Lionel B, Jan 27, 2005
    #8
  9. 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, Jan 28, 2005
    #9
  10. "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:
    www.awprofessional.com/content/downloads/meyerscddemo/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/ThinkingInCPP2e.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
     
    Ivan Vecerina, Jan 28, 2005
    #10
  11. >
    > 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:
    > www.awprofessional.com/content/downloads/meyerscddemo/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/ThinkingInCPP2e.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



    Thanks,
    I will check out next week. Have a gr8 weekend

    Thanks
    Ram
     
    Ramprasad A Padmanabhan, Jan 28, 2005
    #11
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. John Smith
    Replies:
    2
    Views:
    423
    Ivan Vecerina
    Oct 6, 2004
  2. Replies:
    11
    Views:
    735
    James Kanze
    May 16, 2007
  3. hurcan solter
    Replies:
    3
    Views:
    733
    Cholo Lennon
    Aug 29, 2007
  4. Replies:
    11
    Views:
    562
  5. Replies:
    2
    Views:
    312
Loading...

Share This Page