[trac583] addressed review comments

Fixed some style issues, made the singleton instance a static object, and added a bit of documentation

src/lib/asiolink/io_fetch.cc

@@ -67,7 +67,7 @@ IOFetch::operator()(error_code ec, size_t length) {
         {
             Message msg(Message::RENDER);
             
-            msg.setQid(QidGenerator::getInstance()->generateQid());
+            msg.setQid(QidGenerator::getInstance().generateQid());
             msg.setOpcode(Opcode::QUERY());
             msg.setRcode(Rcode::NOERROR());
             msg.setHeaderFlag(Message::HEADERFLAG_RD);

src/lib/asiolink/qid_gen.cc

@@ -23,40 +23,34 @@
 #include <sys/time.h>
 
 namespace {
-    asiolink::QidGenerator* qid_generator_instance = NULL;
+    asiolink::QidGenerator qid_generator_instance;
 }
 
 namespace asiolink {
-QidGenerator*
-QidGenerator::getInstance() {
-    if (!qid_generator_instance) {
-        qid_generator_instance = new QidGenerator();
-        qid_generator_instance->seed();
-    }
 
-    return qid_generator_instance;
+QidGenerator&
+QidGenerator::getInstance() {
+    return (qid_generator_instance);
 }
 
-void
-QidGenerator::cleanInstance() {
-    delete qid_generator_instance;
-    qid_generator_instance = NULL;
+QidGenerator::QidGenerator() : dist_(0, 65535),
+                               vgen_(generator_, dist_)
+{
+    seed();
 }
 
-QidGenerator::QidGenerator() : dist(0, 65535), vgen(generator, dist) {}
-
 void
 QidGenerator::seed() {
     struct timeval tv;
     gettimeofday(&tv, 0);
     long int seed;
     seed = (tv.tv_sec * 1000000) + tv.tv_usec;
-    generator.seed(seed);
+    generator_.seed(seed);
 }
 
 isc::dns::qid_t
 QidGenerator::generateQid() {
-    return vgen();
+    return (vgen_());
 }
 
 } // namespace asiolink

src/lib/asiolink/qid_gen.h

@@ -18,8 +18,8 @@
 // (and other parts where we need randomness, perhaps another thing
 // for a general libutil?)
 
-#ifndef __QID_GEN_
-#define __QID_GEN_
+#ifndef __QID_GEN_H
+#define __QID_GEN_H
 
 #include <dns/message.h>
 #include <boost/random/mersenne_twister.hpp>
@@ -40,15 +40,17 @@ class QidGenerator {
 public:
     /// \brief Returns the singleton instance of the QidGenerator
     ///
-    /// Initializes a new instance if there is none.
-    static QidGenerator* getInstance();
+    /// Returns a reference to the singleton instance of the generator
+    static QidGenerator& getInstance();
 
-    /// \brief Cleans up the singleton instance.
+    /// \brief Default constructor
     ///
-    /// This is added so that we can run memory checkers and
-    /// not get complaints about leaking data. If getInstance()
-    /// is called after cleanInstance, a new one would be created.
-    static void cleanInstance();
+    /// It is recommended that getInstance is used rather than creating
+    /// separate instances of this class.
+    /// 
+    /// The constructor automatically seeds the generator with the
+    /// current time.
+    QidGenerator();
 
     /// Generate a Qid
     ///
@@ -57,18 +59,27 @@ public:
 
     /// \brief Seeds the QidGenerator (based on the current time)
     ///
-    /// This is automatically called when getInstance() is called
-    /// the first time.
+    /// This is automatically called by the constructor
     void seed();
     
 private:
-    QidGenerator();
-    boost::mt19937 generator;
-    boost::uniform_int<> dist;
-    boost::variate_generator<boost::mt19937&, boost::uniform_int<> > vgen;
+    // "Mersenne Twister: A 623-dimensionally equidistributed 
+    // uniform pseudo-random number generator", Makoto Matsumoto and 
+    // Takuji Nishimura, ACM Transactions on Modeling and Computer 
+    // Simulation: Special Issue on Uniform Random Number Generation, 
+    // Vol. 8, No. 1, January 1998, pp. 3-30. 
+    //
+    // mt19937 is an implementation of one of the pseudo random
+    // generators described in this paper.
+    boost::mt19937 generator_;
+    
+    // For qid's we want a uniform distribution
+    boost::uniform_int<> dist_;
+    
+    boost::variate_generator<boost::mt19937&, boost::uniform_int<> > vgen_;
 };
 
 
 } // namespace asiolink
 
-#endif // __QID_GEN_
+#endif // __QID_GEN_H

src/lib/asiolink/tests/qid_gen_unittest.cc

@@ -39,14 +39,10 @@
 
 // Check that getInstance returns a singleton
 TEST(QidGenerator, singleton) {
-    asiolink::QidGenerator* g1 = asiolink::QidGenerator::getInstance();
-    asiolink::QidGenerator* g2 = asiolink::QidGenerator::getInstance();
+    asiolink::QidGenerator& g1 = asiolink::QidGenerator::getInstance();
+    asiolink::QidGenerator& g2 = asiolink::QidGenerator::getInstance();
 
-    EXPECT_TRUE(g1 == g2);
-
-    asiolink::QidGenerator::cleanInstance();
-    // Is there any way to make sure a newly allocated one gets
-    // a new address?
+    EXPECT_TRUE(&g1 == &g2);
 }
 
 TEST(QidGenerator, generate) {
@@ -55,9 +51,9 @@ TEST(QidGenerator, generate) {
     // test (http://xkcd.com/221/), and check if three consecutive
     // generates are not all the same.
     isc::dns::qid_t one, two, three;
-    asiolink::QidGenerator* gen = asiolink::QidGenerator::getInstance();
-    one = gen->generateQid();
-    two = gen->generateQid();
-    three = gen->generateQid();
+    asiolink::QidGenerator& gen = asiolink::QidGenerator::getInstance();
+    one = gen.generateQid();
+    two = gen.generateQid();
+    three = gen.generateQid();
     ASSERT_FALSE((one == two) && (one == three));
 }