The two method below are redundant. We can eliminate the first one by adding a 
default parameter to the second one [orb_name = 0]. 

int init (int &argc, char *argv[]);

int init (int &argc, char *argv[], const char *orb_name);

To Simplify things, we can use CORBA::PolicyList_Destroyer instead of 
CORBA::PolicyList, that way destroy() will be called on the Policy Objects 
even in the case of exceptions, and also we wouldn't need the for loop to 
explicitly destroy the policies.

TAO_ORB_Manager::init_child_poa (int& argc,
                                 char **argv,
                                 const char *poa_name,
                                 const char *orb_name)
{
.
.
.
  // Create the default policies - user-supplied ID, and persistent
  // objects.
  CORBA::PolicyList policies (2);
  policies.length (2);

  // Id Assignment policy
  policies[0] =
    this->poa_->create_id_assignment_policy (PortableServer::USER_ID);

  // Lifespan policy
  policies[1] =
    this->poa_->create_lifespan_policy (PortableServer::PERSISTENT);

  // We use a different POA, otherwise the user would have to change
  // the object key each time it invokes the server.

  this->child_poa_ =
    this->poa_->create_POA (poa_name,
                            this->poa_manager_.in (),
                            policies);

  // Warning!  If create_POA fails, then the policies won't be
  // destroyed and there will be hell to pay in memory leaks!

  // Creation of the new POAs over, so destroy the Policy_ptr's.
  for (CORBA::ULong i = 0;
       i < policies.length ();
       ++i)
    {
      CORBA::Policy_ptr policy = policies[i];
      policy->destroy ();
    }
}


If we use PolicyList_Destroyer then the comment below in the code no longer 
applies.
  // Warning!  If create_POA fails, then the policies won't be
  // destroyed and there will be hell to pay in memory leaks!

Created attachment 752 [details]
Proposed new implementation

Created attachment 753 [details]
Proposed new implementation

The attachments above are the proposed new implemenation (ORB_Manager.cpp/h).

The following is the Change Log Entry:
Sun Apr  8 16:35:14 UTC 2007  Abdullah Sowayan  <abdullah.sowayan@lmco.com>

        * tao/Utils/ORB_Manager.h:
        * tao/Utils/ORB_Manager.cpp:

          Layout Changes.
          Use PolicyList_Destroyer to destroy Policy object even in 
exceptional cases.
          Removed redundant method.
          Use ::_nil() instead of 0 (NULL pointer).

Johnny,

Can you review my changes when you have a chance?

Thanks,
Abdul

To Johnny for review

Abdul, can you make a unified diff of the changes, easier to review. Btw, the activate method seems to have an useless return value. If so, zap the return value and make it void

Johnny,

Yes, some methods do have useless return values indeed. I considered removing them (i.e., changing the return values to void and updating the documentation), but that would change the API, which could potentially break someone's code. A user might have not looked into the code and assumed that -1 would be returned in 

consider the following psuedo-code

if( orb_manager.method1() != 0 )
{
  log an error message and exit;
}

We can zap return values (and live with the fact that some users potentially might have to modify their code), that will make this class cleaner. Let me know if that is acceptable, and I shall do this and submit a uniffied diff for you to review.

Thanks,
Abdul 

added depends on

corrected depends

Created attachment 759 [details]
Proposed new implementation of ORB_Manager.cpp

Created attachment 760 [details]
Proposed new implementation of ORB_Manager.h

Johnny,

I didn't zap the return values, I'll do that when I get a response to my earlier question about the issue of potentially breaking user code. So, such a change (if desired) will be in the next round of improvements.

Also, (i'm brain storming here), instead of taking char as a paramter or return value, shouldn't we be using ACE_TCHAR. I don't usually use wide charachters, but shouldn't this class accomodate it? If so, this is a todo in the next round of improvements.

Thanks,
Abdul

patches are ok, feel free to commit after x.5.8 is out.

remove myself as cc

Johnny,

Thanks for reviewing the patches. I shall commit them once x.5.8 is out.

Thanks,
Abdul

Fri May 11 11:36:25 UTC 2007  Abdullah Sowayan  <abdullah.sowayan@lmco.com>

        * tao/Utils/ORB_Manager.h:
        * tao/Utils/ORB_Manager.cpp:

          Layout Changes.
          Use PolicyList_Destroyer to destroy Policy object even in exceptional cases.
          Removed redundant method.
          Use ::_nil() instead of 0 (NULL pointer).