ChangeLogTag:Sat Sep 28 16:18:08 2002 Ossama Othman <ossama@uci.edu>

1 files changed, 46 insertions, 4 deletions

diff --git a/TAO/examples/AMH/Sink_Server/mt_server.cpp b/TAO/examples/AMH/Sink_Server/mt_server.cpp
index 119d8125b53..48e82f21664 100644
--- a/TAO/examples/AMH/Sink_Server/mt_server.cpp
+++ b/TAO/examples/AMH/Sink_Server/mt_server.cpp
@@ -9,23 +9,56 @@
 void
 usage (char *message)
 {
-  static const char * usage = 
+  // @@ Mayur, what use is there in placing a space before a newline
+  //    character?
+  //
+  // @@ Mayur, have you considered using string concatenation to make
+  //    the following usage message more readable?  For example:
+  //
+  //      static const char * usage =
+  //        "invoke as: mt_server -o <ior_output_file>\n"
+  //        "-n <num_threads>\n"
+  //        "-s <sleep_time (in microseconds)>\n";
+
+  static const char * usage =
   "invoke as: mt_server -o <ior_output_file> \n -n <num_threads>\n -s <sleep_time (in microseconds)> \n";
 
+  // @@ Mayur, why don't you just place the usage message directly in
+  //    the below ACE_ERROR macro?  It's not a big deal.  It's just
+  //    something we normally do.
   ACE_ERROR ((LM_ERROR, "%s : %s", message, usage));
 }
 
+// @@ Mayur, please do not program like a Java programmer.  Please
+//    move the class declaration to a separate header file, and keep
+//    the implementation code in another file preferrably separate
+//    from this one.
 class MT_AMH_Server
   : public Base_Server
   , public ACE_Task_Base
 {
 public:
+  // @@ Mayur, have you considered passing in argc by reference
+  //    instead of as a pointer?  It would save you the indirection
+  //    code in below, and make the code cleaner.  This is what
+  //    ORB_init() does.  Not a big deal in any case.
   MT_AMH_Server (int* argc, char **argv)
     : Base_Server (argc, argv) {}
 
+  // @@ Mayur, empty parameter lists should be denoted with "(void)",
+  //    not "()".  Again, this is detailed in the guidelines.
+  //
+  // @@ Mayur, please put inlined methods in a separate `.inl' file,
+  //    as detailed in the ACE/TAO coding/style guidelines, and as per
+  //    our conventions.
   ~MT_AMH_Server () {}
 
-  // We need to parse an extra thread_count paramter for multi-threraded servers
+  // @@ Mayur, please put inlined methods in a separate `.inl' file,
+  //    as detailed in the ACE/TAO coding/style guidelines, and as per
+  //    our conventions.
+
+  // We need to parse an extra thread_count paramter for
+  // multi-threraded server.
   virtual int parse_args (void)
   {
 
@@ -70,6 +103,9 @@ public:
     return 0;
   }
 
+  // @@ Mayur, please put inlined methods in a separate `.inl' file,
+  //    as detailed in the ACE/TAO coding/style guidelines, and as per
+  //    our conventions.
   void start_threads (void)
   {
     // Each of this thread runs the event loop
@@ -77,6 +113,10 @@ public:
     this->thr_mgr ()->wait ();
   }
 
+  // @@ Mayur, please put inlined methods in a separate `.inl' file,
+  //    as detailed in the ACE/TAO coding/style guidelines, and as per
+  //    our conventions.
+
   // the service method
   virtual int svc (void)
   {
@@ -107,8 +147,10 @@ main (int argc, char *argv[])
 
   if (servant.parse_args (&argc, argv) != 1)
     {
-        usage (ACE_const_cast (char*, "sleep time unspecified"));
-        ACE_OS::exit (1);
+      // @@ Mayur, why are you casting away the const-ness?  Just make
+      //    your usage() function accept "const char *".
+      usage (ACE_const_cast (char*, "sleep time unspecified"));
+      ACE_OS::exit (1);
     }
 
   amh_server.register_servant (&servant);