Copy Constructor with Template Class

Discussion in 'C++' started by Donovan Parks, Sep 29, 2007.

  1. Hello,

    I am rather lost about how to properly setup a copy constructors and
    operator= with my template class. My class if as follows:

    template<class T> class Image
    {
    private:
    IplImage* imgp;

    public:
    Image(IplImage* img = NULL) { imgp = img; }
    ~Image()
    {
    if(imgp != 0)
    cvReleaseImage(&imgp);
    imgp = NULL;
    }

    IplImage* Ptr() { return imgp; }

    void operator=(IplImage* img)
    {
    imgp = img;
    }

    // copy construction
    Image(const T& rhs);

    T& operator=(const T& rhs);
    };


    template<class T>
    Image<T>::Image(const T& rhs)
    {
    if(rhs.imgp != NULL && rhs.m_bReleaseMemory)
    cvReleaseImage(&rhs.imgp);

    imgp = cvCreateImage(cvGetSize(rhs.Ptr()), rhs.Ptr().depth ,
    rhs.Ptr().nChannels);
    cvCopyImage(rhs.Ptr(), imgp);
    }

    template<class T>
    T& Image<T>::eek:perator=(const T& rhs)
    {
    if ( &rhs == this )
    return *this;

    if(imgp != NULL &&m_bReleaseMemory)
    cvReleaseImage(&imgp);

    imgp = cvCreateImage(cvGetSize(rhs.Ptr()), rhs.Ptr().depth ,
    rhs.Ptr().nChannels);
    cvCopyImage(rhs.Ptr(), imgp);
    }

    I am confused about two points. First, although I can compile the
    above code, I can not set a break point in either the
    Image<T>::Image(const T& rhs) or T& Image<T>::eek:perator=(const T& rhs)
    functions. My compiler (VS 8.0) indicates the break points will never
    be hit. Secondly, is I am uncertain if they is sufficient to solve the
    following problem:

    vector< Image<unsigned char> > v;

    void main()
    {
    void Foo();
    v.at(0).size; // error: memory has already been freed!
    }

    void Foo()
    {
    Image<unsigned char> image;
    IplImage iplImage = new IplImage();
    image = iplImage;

    v.push_back(image);
    }

    Obviously this will not work since I am allocating "Image<unsigned
    char> image" on the stack. It will thus free its memory when the
    function ends. I believe that properly overloading the copy
    constructor and operator= will solve this problem, but am not sure how
    to do this.

    Thanks you for any and all help.

    Cheers!
     
    Donovan Parks, Sep 29, 2007
    #1
    1. Advertising

  2. * Donovan Parks:
    > Hello,
    >
    > I am rather lost about how to properly setup a copy constructors and
    > operator= with my template class. My class if as follows:
    >
    > template<class T> class Image
    > {
    > private:
    > IplImage* imgp;
    >
    > public:
    > Image(IplImage* img = NULL) { imgp = img; }
    > ~Image()
    > {
    > if(imgp != 0)
    > cvReleaseImage(&imgp);
    > imgp = NULL;
    > }
    >
    > IplImage* Ptr() { return imgp; }
    >
    > void operator=(IplImage* img)
    > {
    > imgp = img;
    > }
    >
    > // copy construction
    > Image(const T& rhs);
    >
    > T& operator=(const T& rhs);
    > };
    >
    >
    > template<class T>
    > Image<T>::Image(const T& rhs)
    > {
    > if(rhs.imgp != NULL && rhs.m_bReleaseMemory)
    > cvReleaseImage(&rhs.imgp);


    Are you sure you want to change the "rhs" object? This looks like
    changing the object.


    > imgp = cvCreateImage(cvGetSize(rhs.Ptr()), rhs.Ptr().depth ,
    > rhs.Ptr().nChannels);
    > cvCopyImage(rhs.Ptr(), imgp);
    > }


    It seems that you assume that "rhs" is of type Image.

    In that case, the templating isn't actually /used/ for anything.

    Why is this class a template?

    Try to remove the templatizing.

    That may help.



    > template<class T>
    > T& Image<T>::eek:perator=(const T& rhs)
    > {
    > if ( &rhs == this )
    > return *this;
    >
    > if(imgp != NULL &&m_bReleaseMemory)
    > cvReleaseImage(&imgp);
    >
    > imgp = cvCreateImage(cvGetSize(rhs.Ptr()), rhs.Ptr().depth ,
    > rhs.Ptr().nChannels);
    > cvCopyImage(rhs.Ptr(), imgp);
    > }


    You can express assignment in terms of copy construction. Google up
    examples (they involve defining a swap operation).



    > I am confused about two points. First, although I can compile the
    > above code, I can not set a break point in either the
    > Image<T>::Image(const T& rhs) or T& Image<T>::eek:perator=(const T& rhs)
    > functions. My compiler (VS 8.0) indicates the break points will never
    > be hit. Secondly, is I am uncertain if they is sufficient to solve the
    > following problem:
    >
    > vector< Image<unsigned char> > v;
    >
    > void main()


    "main" must have result type "int".


    > {
    > void Foo();


    This declares a function Foo, it doesn't call it.


    > v.at(0).size; // error: memory has already been freed!


    No, it has never been allocated.

    Cheers, & hth.,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Sep 29, 2007
    #2
    1. Advertising

  3. Donovan Parks

    James Kanze Guest

    On Sep 29, 5:26 am, Donovan Parks <> wrote:
    > I am rather lost about how to properly setup a copy
    > constructors and operator= with my template class.


    The first thing to do is to define the semantics of copying, and
    what a copy is, with respect to the original. (Assignment
    usually follows copy.) You should even ask yourself if copying
    makes sense; it doesn't for a lot of things.

    > My class if as follows:


    > template<class T> class Image
    > {
    > private:
    > IplImage* imgp;


    > public:
    > Image(IplImage* img = NULL) { imgp = img; }
    > ~Image()
    > {
    > if(imgp != 0)
    > cvReleaseImage(&imgp);
    > imgp = NULL;
    > }


    > IplImage* Ptr() { return imgp; }


    > void operator=(IplImage* img)
    > {
    > imgp = img;
    > }


    > // copy construction
    > Image(const T& rhs);


    > T& operator=(const T& rhs);
    > };


    > template<class T>
    > Image<T>::Image(const T& rhs)
    > {
    > if(rhs.imgp != NULL && rhs.m_bReleaseMemory)
    > cvReleaseImage(&rhs.imgp);


    > imgp = cvCreateImage(cvGetSize(rhs.Ptr()), rhs.Ptr().depth ,
    > rhs.Ptr().nChannels);
    > cvCopyImage(rhs.Ptr(), imgp);
    > }


    Not knowing the exact semantics of the functions involved, it's
    hard to say, but it sort of looks like you first release the
    image, and then try to copy it. Which, of course, creates two
    problems: you're actually modifying the logical state of the
    object being copied (which a copy constructor shouldn't normally
    do), and you're trying to copy data that has been released
    (freed?), which is probably undefined behavior.

    Depending on the desired semantics of Image, you should do one
    of the following:

    -- declare the copy constructor and the assignment operator
    private, and don't implemnent them,

    -- do a deep copy of the data, without modifying them in the
    object being copied, or

    -- use some sort of reference counting (perhaps make imgp a
    boost::shared_ptr, for example).

    > template<class T>
    > T& Image<T>::eek:perator=(const T& rhs)
    > {
    > if ( &rhs == this )
    > return *this;


    > if(imgp != NULL &&m_bReleaseMemory)
    > cvReleaseImage(&imgp);


    > imgp = cvCreateImage(cvGetSize(rhs.Ptr()), rhs.Ptr().depth ,
    > rhs.Ptr().nChannels);
    > cvCopyImage(rhs.Ptr(), imgp);
    > }


    > I am confused about two points. First, although I can compile
    > the above code, I can not set a break point in either the
    > Image<T>::Image(const T& rhs) or T& Image<T>::eek:perator=(const
    > T& rhs) functions. My compiler (VS 8.0) indicates the break
    > points will never be hit.


    If you don't use a template function, it won't be instantiated,
    and the debugger will find it very difficult to set a breakpoint
    in a function which doesn't exist. Regardless of what
    compiler/debugger you're using.

    If you do use the functions in question, it's really a question
    specific to the compiler/debugger combination, and you'd have to
    ask for more information in a group dedicated to your platform.

    > Secondly, is I am uncertain if they is sufficient to solve the
    > following problem:


    > vector< Image<unsigned char> > v;
    >
    > void main()
    > {
    > void Foo();
    > v.at(0).size; // error: memory has already been freed!
    > }


    What memory? As far as I can see, you've created an empty
    vector. At this point, it's very likely (but not at all
    guaranteed) that none of the functions in Image have been
    instantiated.

    > void Foo()
    > {
    > Image<unsigned char> image;
    > IplImage iplImage = new IplImage();
    > image = iplImage;


    > v.push_back(image);
    > }


    Note that this function is never called in your example code.
    This is probably what causes the error message in your debugger;
    the compiler (or linker) detects that it is never called, so
    removes it, then recursively detects that the template
    instantiations it invokes are not used, and removes them.

    > Obviously this will not work since I am allocating
    > "Image<unsigned char> image" on the stack.


    Which is normal if the type supports copy and assignment (i.e.
    has value semantics).

    > It will thus free its memory when the function ends.


    But you will have copied it into v before that. So if the copy
    constructor works correctly (either deep copy or reference
    counting), the element in v will have the data it needs.

    > I believe that properly overloading the copy constructor and
    > operator= will solve this problem, but am not sure how to do
    > this.


    Correctly defining copy and assignment are necessary if the
    object type is to be contained in a standard container. How
    (and if) they should be defined depends on the semantics of the
    class.

    Just a hint: before writing a single line of implementation code
    for the class, document it. Above the class definition, write a
    comment stating the role, the responsibilities and the
    invariants of the class. Above each fonction, write a comment
    specifying the pre- and post-conditions. Unless you know
    exactly what the class should do, and the pre- and
    post-conditions for each function, it's impossible to even speak
    about whether the code is correct.

    --
    James Kanze (GABI Software) email:
    Conseils en informatique orientée objet/
    Beratung in objektorientierter Datenverarbeitung
    9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
     
    James Kanze, Oct 1, 2007
    #3
    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. christopher diggins
    Replies:
    16
    Views:
    756
    Pete Becker
    May 4, 2005
  2. Replies:
    9
    Views:
    775
  3. ali
    Replies:
    4
    Views:
    579
    David Harmon
    Mar 5, 2007
  4. Generic Usenet Account
    Replies:
    10
    Views:
    2,246
  5. cinsk
    Replies:
    35
    Views:
    2,613
    James Kanze
    Oct 11, 2010
Loading...

Share This Page