dynamic casting?

[leeand00]

Hi,

I am attempting to implement a decorator pattern (http://en.wikipedia.org/wiki/Decorator_pattern) to keep track of the size of something that I am passing over a socket in the variable called PlainServerCall->ip.

(see full code listing bellow)

#include <iostream>
#include <string>

using namespace std;
 
/* Component (interface) */
class ServerCall {
 
public: 
  virtual void draw() = 0; 
  virtual ~ServerCall() {}
};  
 
/* ConcreteComponent */
class PlainServerCall : public ServerCall {
 
private:                  
      

protected:
	
	

public:

   int ip;
   PlainServerCall(){ 

	ip = 0;
   }

   void setIp(int IP) {ip = IP;}
   int getIp() {return ip;}

   void draw() { 
      cout << "PlainServerCall: " << '\n'; 
   }

};
 
/* Decorator (interface) */                                           
class ServerCallDecorator : public ServerCall {
 
private:
protected:
     ServerCall* wid;       // reference to ServerCall
 
public:
   ServerCallDecorator( ServerCall* w )  { 
     wid = w; 
   }
 
   void draw() { 
     wid->draw(); 
   }
 
   ~ServerCallDecorator() {
     delete wid;
   }
};
 
/* ConcreteDecoratorA */
class StringServerCallDecorator : public ServerCallDecorator { 
private:
       	char *arg;
public:
   StringServerCallDecorator(char *strArg, ServerCall* w ) : ServerCallDecorator( w ) 
   { 
	arg = strArg;
   }
   void draw() {
      ServerCallDecorator::draw();    
      cout << "   StringServerCallDecorator: " << arg << ";"; 
      
      PlainServerCall *xx_srv_call = dynamic_cast<PlainServerCall *>( wid );
      xx_srv_call->setIp(xx_srv_call->getIp()+strlen(arg)+1);

      cout << " ip: " << xx_srv_call->getIp() << endl;
   }  
};
 
/* ConcreteDecoratorB */
class ShortServerCallDecorator : public ServerCallDecorator {
private:
	short *arg;	
public:
   ShortServerCallDecorator(short *shortArg, ServerCall* w ) : ServerCallDecorator( w ) 
   { 
   	arg = shortArg;
   }

   void draw() {
      ServerCallDecorator::draw(); 
      cout << "   ShortServerCallDecorator: " << *arg << ":";

      PlainServerCall *xx_srv_call = dynamic_cast<PlainServerCall *>( wid );
      xx_srv_call->setIp(xx_srv_call->getIp()+sizeof(arg));

      cout << " ip: " << xx_srv_call->getIp() << endl;
   }  
};
 
int main( void ) {

   char *myStr = new char[20];
   short *myShort = new short;

   *myShort = 1;

   strcpy(myStr, "myStr");

   ServerCall* aServerCall = new StringServerCallDecorator(myStr,
                     new ShortServerCallDecorator(myShort,
                     new PlainServerCall()));
  
   aServerCall->draw();
   delete aServerCall;
}

when I run the program it seems to get to the following point and then dies with a segmentation fault.

It almost seems as though the StringServerCallDecorator class doesn't like anyone accessing it's char * arg being accessed by cout.

StringServerCallDecorator
...
   void draw() {
      ServerCallDecorator::draw();    
      cout << "   StringServerCallDecorator: " << arg << ";";

output:

PlainServerCall:
ShortServerCallDecorator: 1: ip: 4

Segmentation fault

I know this has something to do with pointers. Can someone please tell what I did wrong, I'm new to C++.

Thank you,
Andrew J. Leer

[weaknessforcats]

ServerCall* aServerCall = new StringServerCallDecorator(myStr,
                     new ShortServerCallDecorator(myShort,
                     new PlainServerCall()));

None of the new allocations used as function arguments will ever be deleted becuse you never captured the addresses to delete.

You need to:

PlainServerCall* A = new PlainServerCall;
ShortServerCallDecorator* B = new ShortServerCallDecorator(myShort, A);

ServerCall* aServerCall = new StringServerCallDecorator(myStr, B);

//then at the end...
delete A;
delete B;
delete aServerCall;

But your main problem is here:

void draw() {
      ServerCallDecorator::draw();    
      cout << "   StringServerCallDecorator: " << arg << ";"; 
      
      PlainServerCall *xx_srv_call = dynamic_cast<PlainServerCall *>( wid );
      xx_srv_call->setIp(xx_srv_call->getIp()+strlen(arg)+1);

      cout << " ip: " << xx_srv_call->getIp() << endl;
   }  

You do not check that the dynamic cast actually worked. In this case it does not. The wid is not a PlainServerCall* and the dynamic_cast returns zero which is put into xx_srv_call.

Unfortunately, you do not check xx_srv_call for zero before using it and so you crash right here. Before using the -> indirection operator you must check that the pointer is not zero.

If you are implementing Decorator, you do not need a dynamic_cast. In fact, I will go so far as to say that any cast in a C++ program means a) your are calling a relic C function or b) your C++ design is flawed.

[leeand00]

If I don't cast a ServerCall (interface) to a PlainServerCall, than how am I supposed to get to the variable ip, which I have declared inside of the PlainServerCall class?

[weaknessforcats]

First, your class ServerCall already has a draw() as a pure virtual function. That's OK.
Second, PlainServerCall derives from ServerCall and implements a draw() for a PlainServerCall. That's OK.

From here on you use PlainServerCall objects by referring to them as ServerCall pointers or references.

ServerCall obj = new PlainServerCall;

then this:
obj->draw();

will call PlainServerCall::draw();

Now your ServerCallDecorator does not add any methods beyond those declared in ServerCall. Until it does, it is a useless class and you don't need it. That is, as of now it does decorate your ServerCall class.

However, let's assume you need to add MethodA() sso you can either
draw() or call MethodA(). You would write code so that:

ServerCallDectorator* dec = new SeverCallDecorator(obj);

where obj is a ServerCall*.

then you can:

dec->draw();
dec->MethodA();     //this is the decorator method 

aand to make that work, ServerCallDecorator will contain a pointer to a ServerCall object:

class ServerCallDecorator
{
      private:
         ServerCall* theCall;
      public:
          void draw();
          void MethodA();
};

void ServerCallDecorator::draw()
{
      theCall->draw();
}
void MethodA()
{
       //some logic
}

So nowhere in here do you need a cast of any kind.

[leeand00]

Yeah, you're right man.

I figured out what I was doing wrong after you told me that I really didn't need to cast anything. I realized that what I really wanted to do was return a variable through the calling of the decorators various methods in order to arrive at a sum of the length of the entire variable.

Thanks!