Is object returned from factory method assignable to other object ?

[can-mohan]

Hi All,
In below code when class a object is assigned to class b by returning object through factory method i observed assignment is not happening properly and after assignment still class b's object points to NULL object only.

*cobj=rhs;//it gives NULL object

However below statement works  perfectly when b object cobj is directly assigned inside assignment operator .
cobj=a::fact("a11")//it works fine

can anybody let me know what is wrong with assignment approach ? I know there are other way as well (setter method) but i am curious
to know in this circumstances how assignment is taken care.

#include <iostream>

using namespace std;

class a {

    int a1;

public:

    a(int _a1 = 0) : a1(_a1) {}

    void virtual showname()=0;

    static a* fact(string name);

};

class a11:public a

{

public:

void showname()

{

cout<<"john";

}





};



class a12:public a

{

public:

void showname()

{

cout<<"jack";

}





};







a* a::fact(string name)

{



if(name=="a11")

{



return new a11();

}



if(name=="a12")

{



return new a12();

}

return NULL;

}



class b {

    public:

        const b& operator=(const a& rhs);

    private:

        a* cobj;

      public:

    void fun(){cobj->showname();}

};



const b& b::operator=(const a& rhs) {



if(&rhs!=cobj)

  {

   delete cobj;

   *cobj=rhs;//it gives NULL object

   //cobj=a::fact("a11");it works fine

   }

    return *this;

}



int main() {

    a* aobj = a::fact("a11");

    b* bobj = new b;

    *bobj = *aobj;

    bobj->fun();

    return 0;

}

[AssertNull]

I'm not familiar with the Factory Method, so I won't offer advice on how to implement that. You're getting the seg fault because you are deleting cobj, then trying to dereference the deleted object. That won't work.

As for what WILL work, if you simply delete line 127, that should result a call to a default assignment operator for class a in line 129, since you haven't written your own. aobj is of type a11, not type a, so if you leave everything to the automatically-written-by-compiler assignment operators, you might not get what you want. You might want to either experiment around with some casting and/or writing some custom assignment operators and/or constructors/destructors. Pay attention to the warning on line 127. You need to write some destructors anyway so you can delete cobj. That's independent of any assignment operator problems. You could then perhaps delete cobj as needed and then call rhs's copy constructor and assign it to cobj.

So either use "delete" followed by "new" with a call to rhs's copy constructor, or don't use delete and use some casting along with a call to rhs's assignment operator.

[can-mohan]

Thnaks for your observation. delete should be there as i need to set diff-2 object over there which is returned from factory function. I agree that inside b's destructor cobj needs to be deleted .Now the question is what is the efficient way to implement the assignment opreator here.
i can't do : cobj= new a (rhs); because a is a abstract class and can't be instantiated.

what i tried:
I tried cobj=rhs but getting error: **cannot convert const a to a* in assignment*
ususally factory function return heap allocated object itself so inside assignment operator new operator need not be used. Now the question is what casting needs to be done here in order to collect factory object inside assignment operator?

[AssertNull]

Typecasting is not something I am good at, so I'll offer no advice. I know just enough to be dangerous. I can usually rig something up that works, then I test it, and if it passes testing, I'll go with it. Works for me, mostly, but I don't want to contribute to any bad practices, so I'll just point you to what appears (to me) to be a good tutorial. I am going to give it a try myself.

http://www.cplusplus.com/doc/tutorial/typecasting/

Now, the reason for my posting (i.e. something where I think I know what I'm doing and I can actually offer some useful advice :))...

i can't do : cobj= new a (rhs); because a is a abstract class and can't be instantiated.

I'll reiterate my earlier post. Line 129's syntax, as you originally had it, is a legitimate call to a's assignment operator.

*cobj=rhs;

Again, if you keep line 127 commented out, there is no deletion of cobj, so you want line 129 to be a call to an assignment operator, not a call to "new". The problem, as mentioned earlier, is that this is going to call the assignment operator for cobj's type, not rhs's type. Hence the need for some dynamic casting or whatever.

Now, you appear to want to create a new cobj object on line 129, but you're getting the syntax wrong, as mentioned. If you go this route, you need to delete the old cobj to prevent a memory leak. So you must uncomment line 127.

Note the warning (warning, not error) when you uncomment that line...

warning: deleting object of abstract class type 'a' which has non-virtual destructor will cause undefined behaviour [-Wdelete-non-virtual-dtor]

You need to write some destructors or change the polymorphism or analyze the warning and decide that the it doesn't matter and ignore it or deal with it in some other way.

OK, back to line 129. You've decided to create a new object rather than try for a call to a11's assignment operator. I see no a11 or a12 copy constructors, so you'd have to write them. I do have a call to dynamic_cast in the code below. Remember my comment about casting (i.e. I'm not that great at it, so read up on pitfalls, exception handling, etc., and implement if needed). Seems OK here though. You can use typeid to check rhs's type at runtime, then send it to the correct block of code to create a new object for cobj to point to, one block of code for each derived class of a (in this case, that's two, one for a11, one for a12).

const b& b::operator=(const a& rhs) 
{
    if (&rhs != cobj) 
    {
        delete cobj;
        if(typeid(a11) == typeid(rhs))
        {
            const a11* temp = dynamic_cast<const a11*>(&rhs);
            cobj = new a11(*temp);
        }
        else if(typeid(a12) == typeid(rhs))
        {
            const a12* temp = dynamic_cast<const a12*>(&rhs);
            cobj = new a12(*temp);
        }
        else
        {
            // should never get here?
        }
    }
    return *this;
}

[can-mohan]

+1 for your analysis . it looks fine for me , only below condition needs to be taken
care . i will see if function try block can be used .

else
        {
            //return NULL;
            // should never get here?
        }