Solved

design guidance

Posted on 2003-11-14
25
505 Views
Last Modified: 2013-12-03

// MessageArray.h
template <class T>
class MessageArray
{
public:
  void store(const T&);
  T retrieve();

private:
  T rcvdMsg [ 2 ];
  int countIndex;
  int currentIndex;
};

//  class 2
class FOO {
public:
  FOO() {}
 ComputeMe () {  std::cout << "  compute me called " << std::endl; }
private:

};

// class 3 - Main class

#include "MessageArrary.h"
#include "foo.h"

class MainClass {
public:
  MainClass ();  
  MessageArray <RCVD_MSG1> msg1;   // now we're dealing with composition
  // lots more MessageArrays

  void CallComputeMe();
private:
};

// main_class.cpp

FOO  ptrFoo = NULL;
MainClass::MainClass()
{
  // here I'd like to create an instance of FOO so I could do
    ptrFoo = new FOO;

}
void MainClass::CallComputeMe()
{
   ptrFoo->ComputeMe();   // now call compute me.
   msg1.Store ( ... );
}

////////////
1.  In terms of composition, wouldn't I be better off putting  'MessageArray <RCVD_MSG1> msg1';  inside the constructor of MainClass?  Just trying to garner a feel for the pros and cons since the constructor for MessageArray gets called before that of MainClass.
2.  In terms of creating an instance of FOO for use in MainClass.  Does the current implementation 'seem' efficient?  Just trying to get a feel for a better approach .. in this context which lacks inheritance.
3.  What s the use of 'public' being define twice here?

class FOO
{
  public :
   // some stuff
  public:
  //  some other stuff

};

I see code with this and i'm unsure what the impetus behind 'public, public' is

4.  If I had
class One {
public:
   void printStuff ();
   void Initialize();
};

class Two {
  void Startup();
  void AA();
 // lots more

};

Background info:  The member functions of class One serve as a base class for class Two. In other words, printStuff - for instance - in class One is really printing 'stuff' inside class Two - the worker class.
Trying to convince some folks that we'd be better off using class One as a base with class Two derived and usign the polymorphic aspects of ++ where we'd use a base class (virtuals) to access a derived.  Makes Sense?  Thus far the plan is to main separate classes which seems silly to me.  Of course we're no expert (just enough to be dangerous) in ++ so hence the hesitation.  
0
Comment
Question by:forums_mp
  • 13
  • 12
25 Comments
 
LVL 15

Expert Comment

by:efn
ID: 9753114
> 1.  In terms of composition, wouldn't I be better off putting  'MessageArray <RCVD_MSG1> msg1';  inside the constructor of MainClass?

I'm guessing you mean something like this:

MainClass::MainClass()
{
   MessageArray <RCVD_MSG1> msg1;
}

If that's what you meant, no, it wouldn't be better.  You apparently want to use msg1 as a member, and if it's local to the constructor, it will disappear when control leaves the constructor.  If you meant something else (maybe dynamically allocating it in the constructor?), please clarify.

> 2.  In terms of creating an instance of FOO for use in MainClass.  Does the current implementation 'seem' efficient?

It seems reasonably efficient.  It might be slightly more efficient to have a FOO member, but there could also be reasons for allocating it dynamically.  With abstracted sample code like this, it's hard to tell.  But there is nothing wrong in general with the approach you have.

> 3.  What s the use of 'public' being define twice here?

It saves the programmer the time required to produce neat and tidy code.  Functionally, it is pointless.

> 4. ... Makes Sense?

It's hard to tell from the information you presented.  The classes should correspond to some meaningful concepts.  The classical guideline is that inheritance should model an "is a" relationship.  Is every Two also a One?  Is a Two a kind of One?  Might there be other kinds of One some day?  Will it be useful to be able to treat a Two as a One sometimes?  If a Two just works for a One, but is not itself a kind of One, inheritance might not be helpful.

--efn
0
 

Author Comment

by:forums_mp
ID: 9754005

efn.  Consider this small "C" snippet we'd like to transform to C++

// aa.h
#ifndef  AA_H
#define AA_H

  typedef struct _MY_TX_MSG
  {
     // lots of goodies.
  }  MY_TX_MSG;

  typedef struct _HEADER
  {
     unsigned int msg_count     : 16;
     unsinged int msg_id          : 16;
  }  HEADER;


  typedef struct _MY_MSG
  {
     HEADER   header;
  }  MY_MSG;

#endif
-------------------------------------------------------------------------
// foo.h
#ifndef  FOO_H
#define FOO_H

#include "aa.h"

  int tx_error_count;
  int rx_error_count;

  int tx_count;
  int rx_count;

  MY_TX_MSG my_tx_msg[2];
  MY_MSG my_msg;

 // functions used in foo.c
  void Print();      
  void Initialize();
  void HeaderInitialize();

  // functions used in foo_worker.c
  void DoWork();  
  // etc.
#endif;
-------------------------------------------------------------------------
// in foo.c  
#include "foo.h"
void Initialize ()
{
  tx_error_count  = 0;
  rx_error_count = 0;

  tx_count          = 0;
  rx_count          = 0;
}

void Print()
{
  printf (" tx_error_count = %d\n ", tx_error_count);
  printf (" rx_error_count = %d\n ", tx_error_count);

  printf (" tx_count = %d \n",  tx_count);
  printf (" rx_count = %d \n", rx_count);
 
  memset (my_tx_msg, 0, sizeof (MY_TX_MSG) * 2);  // common_buffer replaces this ..
  HeaderInitialize();
}

void HeaderInitialize()
{
   my_msg.header.msg_count  =  0;
   my_msg.header.msg_id       =  0;
   // must be an easier way in ++ to initialzie this without having to spell out everything.
}
-------------------------------------------------------------------------
// foo_worker.c  - here all the stuff in  foo.c gets manipulated
#include "foo.h"

void DoWork()
{
 // conditional constructs and thigns not shown.

  tx_error_count++;
  rx_error_count++;

  tx_count++;
  rx_count ++;

// do stuff on the messages
}


-------------------------------------------------------
-------------------------------------------------------

Now to overhaul this in a C++ environment while keeping foo.h, foo.cpp and foo_worker.cpp. My thoughts:

1.  Create two classes in foo.h

class FOO {
  public:
       void Print();      
       void Initialize();
       void HeaderInitialize();
};

class FOO_WORKER {
  public:
     void DoWork();
};

2.
// in foo.cpp
#include "foo.h"

3.
 // in foo_worker.cpp
#include "foo.h"

But wait what about the common data members .. (int tx_count, rx_count etc etc.).  One thought was to create a COMMON_DATA member class with public members but that seems silly besides that'll introduce a new class .. but of course that could become a part of foo.h?
a namespace perhaps .. hmnn!!?

Your thoughts on overhauling this.  I'll be ordering a text on design patterns :)
0
 
LVL 15

Expert Comment

by:efn
ID: 9754549
> Now to overhaul this in a C++ environment while keeping foo.h, foo.cpp and foo_worker.cpp.

My first question is:  why is this constraint needed?  If you are going to be redesigning the code into classes, why can't you have different files?  Maybe you meant to keep the same functionality, not necessarily the files.

I think all the C you originally showed would also work in C++.  So I guess what you are really after is a more object-oriented design.  To pursue that, you have to try to "think in objects," and look for combinations of code and data that make sense conceptually as objects.

In this case, you have messages and headers, both of which could be objects.  You also have counts, which you could package into an object.  The Initialize function looks like it should be either the constructor or a member of the class that contains and manages the counts.  The counts class could also do its own output.  That is, rather than having some other code get the data out of the counts object and display it, the client code could just tell the counts object to expose itself.  The memset on a message looks like a member function of the message class, and similarly, the HeaderInitialize function looks like a member of the header class.  The DoWork function, instead of incrementing counts directly, could tell the counts object which ones to increment.

>    // must be an easier way in ++ to initialzie this without having to spell out everything.

I don't think so.  But you should only have to code it once.

>  that'll introduce a new class

There's nothing wrong with introducing a new class, and if it makes the design make more sense, go for it.  One of the most common design flaws, in my opinion, is letting classes get too big and overgrown.

> I'll be ordering a text on design patterns

I think a book on object-oriented design is what you need.  I'd suggest "Fundamentals of Object-Oriented Design in UML" by Meilir Page-Jones.  "C++ FAQs" by Cline, Lomow, and Girou and "Thinking in C++" by Bruce Eckel are also good.

--efn
0
 

Author Comment

by:forums_mp
ID: 9754676

> Maybe you meant to keep the same functionality, not necessarily the files.

True..  which means in " foo.h"

  // keep all the declarations .. etc.
  int tx_error_count;
  int rx_error_count;

  int tx_count;
  int rx_count;
   // etc
   
// later
class FOO
{
};

class FOO_WORKER
{
};

//end foo.h

>> So I guess what you are really after is a more object-oriented design.  To pursue that, you have to try to "think in objects," and look for combinations of code and data that make sense conceptually as objects.  In this case, you have messages and headers, both of which could be objects.  You also have counts, which you could package into an object.

With this approach
class COUNTS
{
  public:

  int tx_error_count;
  int rx_error_count;

  int tx_count;
  int rx_count;
};
class MESSAGES_HEADERS
{
  MY_TX_MSG my_tx_msg[2];
  MY_MSG my_msg;
};

now
class FOO : public COUNTS, public MESSAGE_HEADERS
{

};

class FOO_WORKER :  public COUNTS, public MESSAGE_HEADERS
{

};

> The counts class could also do its own output.  
Meaning  'void Output()' or some such thing withing COUNTS

> The DoWork function, instead of incrementing counts directly, could tell the counts object which ones to increment
??

--------
Speaking of books.  Eckels I've got a 'minor' confusion on his use of the word object - in my view - interchangeably.   Here's what I think I understand

class FOO
{
};
 
FOO aa;

aa is an object
FOO is also an object.  FOO is supposed to be a user defined type, not an object.  aa is the object  or is it me?


I've got Eckel (good stuff), Cline's in the mail and the other .. i need to order :)
As always thanks
0
 
LVL 15

Expert Comment

by:efn
ID: 9754738
> class FOO : public COUNTS, public MESSAGE_HEADERS

This looks questionable.  Remember, inheritance should generally model an "is a" relationship.  Is a FOO really a kind of COUNTS and a kind of MESSAGES_HEADERS, and is a FOO_WORKER really a different kind of COUNTS and MESSAGES_HEADERS?  That doesn't seem to make sense to me.  I would think you would have just one COUNTS object and a FOO would use it.

> FOO is also an object.  FOO is supposed to be a user defined type, not an object.  aa is the object  or is it me?

You're right, FOO is not an object.

--efn
0
 

Author Comment

by:forums_mp
ID: 9754767

>Remember, inheritance should generally model an "is a" relationship
Yes!! so composition

class FOO {
public:
  COUNTS                    counts;
  MESSAGES_HEADERS msg_headers;

};

// likewise FOO_WORKER
class FOO_WORKER {
public:
  COUNTS                    counts;
  MESSAGES_HEADERS msg_headers;

};
correct?

------------------------------------
In terms of functionality, just keep the globals and inside foo.h, create the two separate classes?
0
 
LVL 15

Assisted Solution

by:efn
efn earned 500 total points
ID: 9754902
> correct?

Sorry, I rather doubt it.  Do you really want each FOO and each FOO_WORKER to have its own COUNTS and MESSAGES_HEADERS?  To get functionality comparable to the original code, I think you want to have one COUNTS and one MESSAGES_HEADERS and have both FOO and FOO_WORKER use these objects.

There are lots of ways to do this.  I'll sketch a simple one.

COUNTS counts;

FOO foo;
FOO_WORKER foow;

foo.yourCountsObjectIs(&counts);
foow.yourCountsObjectIs(&counts);

> In terms of functionality, just keep the globals and inside foo.h, create the two separate classes?

No, avoid global variables if possible, and in header files, only declare classes, don't create anything, but maybe that's what you meant.

--efn
0
 

Author Comment

by:forums_mp
ID: 9755320

One final one (fingers crossed)

>No, avoid global variables if possible, and in header files, only declare classes, don't create anything, but maybe that's what you meant.

basically what you're saying here (based on what you've already said) is use a class to encapsulate the globals defined in the C code.  akin to the example above?

Thanks again
0
 
LVL 15

Accepted Solution

by:
efn earned 500 total points
ID: 9756165
Using one or more classes to encapsulate the variables is a good idea.  A separate issue is how to provide access to them.

Say you have a FOO that you want some client functions to be able to access.  You could make it a global variable.  In a header file, you would have a declaration:

extern FOO george;

and in exactly one implementation file, you would have a definition at file scope:

FOO george;

This is not the most desirable design.  Any function could tamper with george, and anybody who tried to define another variable of any type with the same name would get a name collision.

It's better to have more private objects and introduce them only to those who need to know them, as in my example above, assuming that the definitions are not at file scope, e.g.:

int main(int argc, char* argv[])
{
  COUNTS counts;

  FOO foo;
  FOO_WORKER foow;

  foo.yourCountsObjectIs(&counts);
  foow.yourCountsObjectIs(&counts);
}

With this kind of design, only the main function and those objects that are told about the counts object can access that object.  This is safer and makes the code easier to understand.

There is also a "singleton" pattern, where the design of the class guarantees that there will never be more than one object of the class constructed and the class itself provides access to that one object.  You probably don't need to get that wizardly at this point in your education.

--efn
0
 

Author Comment

by:forums_mp
ID: 9756635

<SNIP>
>> There is also a "singleton" pattern, where the design of the class guarantees that there will never be more than one object of the class constructed and the class itself provides access to that one object.  You probably don't need to get that wizardly at this point in your education
</SNIP>

Singleton!!!  Very interesting you brought this up.  Prior to posting here I was reading about this, and was trying to hunt down an example/appication as it relates to my issue but then uncertainty kicked in.   Can you give me an example of this??
 
Now lets - if I may deal with this in a more concrete example as it relates to our current discussion.  

We know that with C desing FOO initializes variables/structs and prints variables/struct..
FOO_WORKER does the real work on said variables and structs.

transformed to C++

#ifndef  COMMON_H
#define COMMON_H

class COMMON // for now we'll assume he'll handle counts and structs.
{
public:
   struct RCVD_MSG1
   {
      int idx  :    4;
      int  jdx  :   4;
      int  aa  :    4;
      int bb   :    4;
   };

    int tx_count;
    int rx_count;
    MessageArray <RCVD_MSG1> msg1;
   RCVD_MSG1 local;
};
#endif;
------------------------------------------>
#ifndef FOO_H
#define FOO_H

#include "common.h"
class FOO {

public:
// lets assume we'll initialize the data members and stucts in one initialize function
  void Initialize (COMMON& obj)
  {
       obj.tx_count  = 0;
       obj.rx_count  = 0;
       // since msg1 is a class what do i do about 'him'?  
       //  Of course the mere instance of 'him' (above) initializes him so do i care?

     memset(obj.local, 0, (sizeof obj.RCVD_MSG1));
  }
  void Print (COMMON& obj) { }  // same approach as above...
};
#endif

------------------------------------------>
#ifnef FOO_WORKER
#define FOO_WORKER
#include "common.h"

class FOO_WORKER {
public:
  void DoWork (COMMON_BUFFER& aa)
  {
     aa.tx_count++;
     aa.rx_count++;

    // etc
  }
 
};

#endif;
------------------------------------------>
now we're ready

int main(int argc, char* argv[])
{
  COMMON counts;

  FOO foo;
  FOO_WORKER foow;

  foo.Initialize(&counts);    // this shoudl initialize things ..to zero (rx_count and tx_count)
  foow.DoWork(&counts);  // this should set tx_counts and rx_counts to 1.

  return 0;
}

Haven't tried compiling the above but this is basically the approach in a nutshell?

0
 
LVL 15

Assisted Solution

by:efn
efn earned 500 total points
ID: 9757003
Singleton stuff:

http://gethelp.devx.com/techtips/cpp_pro/10min/10min0200.asp

http://www.experts-exchange.com/Programming/Programming_Languages/Cplusplus/Q_20481658.html

http://www.experts-exchange.com/Programming/Programming_Languages/Cplusplus/Q_11398498.html

http://www.experts-exchange.com/Programming/Programming_Languages/Cplusplus/Q_20666924.html

Note that typical implementations like you see in these pages are not thread-safe.  It's possible to have a thread-safe singleton class, but it's more complicated.

>       // since msg1 is a class what do i do about 'him'?

Get him initialized appropriately.

>       //  Of course the mere instance of 'him' (above) initializes him so do i care?

Not if the default initialization leaves the object in an appropriate state.

>     memset(obj.local, 0, (sizeof obj.RCVD_MSG1));

It's best to stay away from memset if you can, as it's a low-level function that trashes the type system.

> Haven't tried compiling the above but this is basically the approach in a nutshell?

It's an approach that will work, but it's not the kind of object-oriented approach I was recommending.

I know there are historical reasons for having foo initialize counts, but I hope you are allowed to improve the design.  To do that, you have to give some thought to the COMMON class and how it will or might be used in the future.  If a COMMON object is something that you think you might want to pass around to different kinds of objects to have them initialize it in different ways, then your current design is appropriate.  If there is some limited set of initializations that are all anybody would ever want, you could build those into the COMMON class as member functions.  And if you expect that whenever you have a COMMON object, it will always be initialized in one standard way for ever more, you should initialize it in the COMMON constructor.  My guess is that the last case is probably what you have, so I would suggest putting the COMMON initialization code in the COMMON constructor, not in the FOO class.

As you noted, the same thing goes for the Print function.  It is common for an object to be responsible for producing a human-readable representation of itself.  Having that code associated with the class rather than some foreign class like FOO helps make the class more encapsulated and reusable.  So I would suggest that the function that puts out a COMMON should be a member of COMMON, not a member of FOO.

The call to foow.DoWork won't work because it passes a pointer to COMMON and the functon expects a reference to COMMON_BUFFER.  (Note that there is a pointer/reference difference as well as a type difference.)  Aside from that, it should work, but again is not what I was suggesting.  Again, you must think about how the classes will be used.  If you might have a bunch of COMMONs floating around and every time you tell a FOO_WORKER to DoWork, you have to tell it which COMMON to use, your design is appropriate.  But if there is just going to be one COMMON and the FOO_WORKER is just going to use it over and over, you can make the design simpler and more efficient by introducing the COMMON to the FOO_WORKER just once and having the FOO_WORKER remember the COMMON.  For example:

class FOO_WORKER
{
  ...
  COMMON* commonPointerMember;
  ...
};


void FOO_WORKER::heresYourCommon(COMMON* pCommon)
{
  commonPointerMember = pCommon;
  return;
}

void FOO_WORKER::DoWork()
{
  use commonPointerMember here to access the one interesting COMMON object
}

In main:

COMMON counts;
FOO_WORKER foow;

// One time:
foow.heresYourCommon(&counts);

//  Probably in a loop:
foow.DoWork();  // foow remembers counts so you don't have to pass it every time.

--efn
0
 

Author Comment

by:forums_mp
ID: 9759328

>>But if there is just going to be one COMMON and the FOO_WORKER is just going to use it over and over, you can make the design simpler and more efficient by introducing the COMMON to the FOO_WORKER just once and having the FOO_WORKER remember the COMMON.

Indeed!!..  I'm intrigued.  For completeness, using your example

// common_header.h

#ifndef COMMON_HEADER_H
#define COMMON_HEADER_H

#include <stdlib.h>
#include <iostream>
#include <string.h>

class COMMON_HEADER
{

private:

public:      //  With this approach ... all public. !!

   int  tx_count;
   int  rx_count;

   struct MY_STRUCT
   {
         int aa  :  4;
         int bb  :  4;
         int cc  :  4;
         int dd  :  4;
   };

   MY_STRUCT my_struct;

  // COMMON_HEADER will have a template class MsgBuffer<MY_STRUCT> my_struct;

   void Print()
   {
        std::cout << " Value of Tx Count _ Common_Buffer = " << tx_count << std::endl;
   }

      COMMON_HEADER() :
        tx_count(0), rx_count(0)
        {
              memset(&my_struct, 0, sizeof MY_STRUCT);
              std::cout << " common_header constructor called " << std::endl;
        };

        ~COMMON_HEADER() { std::cout << " common_header destructor called " << std::endl; }

};

#endif

// foo_worker.h
#ifndef FOO_WORKER_H
#define FOO_WORKER_H

#include <stdlib.h>
#include "common_header.h"

class FOO_WORKER
{

private:
    COMMON_HEADER *foosCommon;   // but we hide 'things'  here.
public:
     FOO_WORKER(COMMON_HEADER *ptr);
   ~FOO_WORKER();
   void DoWork();
};

#endif

// foo_worker.cpp

#include "foo_worker.h"
#include <iostream>

FOO_WORKER::FOO_WORKER(COMMON_HEADER *ptr)  : foosCommon(ptr)
{
      std::cout << "foo_worker constructor called" << std::endl;
}

FOO_WORKER::~FOO_WORKER()
{
   std::cout << "foo_worker  destructor called " << std::endl;
   // should I clean up foosCommon here ????
}

void FOO_WORKER::DoWork()
{
    foosCommon->tx_count++;
      std::cout << " tx_count = " << foosCommon->tx_count << std::endl;
}

// Now we're ready

#include "foo_worker.h"
#include "common_header.h"

int main ()
{
   COMMON_HEADER common;

   FOO_WORKER* ptrFoo = new FOO_WORKER (&common);

   for (int idx = 0; idx < 5; idx++)
   {
     ptrFoo->DoWork();
       common.Print();
   }

   delete ptrFoo;

   return 0;
}

The output..
------------------------------------------
common_header constructor called
foo_worker constructor called
 tx_count = 1
 Value of Tx Count _ Common_Buffer = 1
 tx_count = 2
 Value of Tx Count _ Common_Buffer = 2
 tx_count = 3
 Value of Tx Count _ Common_Buffer = 3
 tx_count = 4
 Value of Tx Count _ Common_Buffer = 4
 tx_count = 5
 Value of Tx Count _ Common_Buffer = 5
foo_worker  destructor called
 common_header destructor called
Press any key to continue

Ideally it'd be good for me to at least override the copy construcotr and assignment operator in COMMON_HEADER and FOO_WORKER ... well come to think of it a copy constructor for FOO_WORKER (with this example) should be fairly straightforward.

I looked at the links (thats what I get for initially not purchasing excels part II)  and thought well, now I could remap the above COMMON_BUFFER to use a singleton design.

0
Do You Know the 4 Main Threat Actor Types?

Do you know the main threat actor types? Most attackers fall into one of four categories, each with their own favored tactics, techniques, and procedures.

 
LVL 15

Expert Comment

by:efn
ID: 9759547
>    // should I clean up foosCommon here ????

No.  A FOO_WORKER just uses the COMMON_HEADER object, it doesn't create or own it, so it shouldn't do anything to it in the FOO_WORKER destructor.  Nothing is necessary to destroy the pointer that the FOO_WORKER has.

Was that your only question?

--efn
0
 

Author Comment

by:forums_mp
ID: 9760471

efn.   Recall an earlier post where you stated:

>>>There is also a "singleton" pattern, where the design of the class guarantees that there will never be more than one object of the class constructed and the class itself provides access to that one object.

Consider
// singleton approach based on one of the links .. above.  I have two "??" marks within  the code.

#ifndef SINGLETON_COMMON_H
#define SINGLETON_COMMON_H

#include <stdlib.h>
#include <iostream>
#include <string.h>

using namespace std;
class Singleton
{
      static Singleton s;
      // int i;
    Singleton() : tx_count(0), rx_count(0)
      {
        memset(&my_struct, 0, sizeof MY_STRUCT);
        std::cout << "singleton_constructor called " << std::endl;
      }

      void operator=(Singleton&);
      Singleton(const Singleton&);

public:
      static Singleton& getHandle()
      {
            return s;
      }

   int  tx_count;
   int  rx_count;
   // lots and lots more of this sort of 'stuff'.  no choice but to
leave
   // them exposed (i.e public) since FOO_WORKER needs access to them.

   struct MY_STRUCT
   {
         int aa  :  4;
         int bb  :  4;
         int cc  :  4;
         int dd  :  4;
   };

   MY_STRUCT my_struct;
   // lots and lots more of this sort of 'stuff'

   // print_tx count for kicks and grins...
   void Print()
   {
        std::cout << " Value of Tx Count _ Singleton_Buffer = " << tx_count
<< std::endl;
   }
      ~Singleton() { std::cout << " singleton_destructor called " <<
std::endl; }   // Note static class so i dont think it knows anything
about destructing ????

      //int getValue() { return i; }
      //void setValue(int x) { i = x; }
};

#endif;

// foo_worker.h
#ifndef FOO_WORKER_H
#define FOO_WORKER_H

#include <stdlib.h>
#include "singleton_common.h"

class FOO_WORKER
{

private:
   Singleton *foosCommon;

public:

   FOO_WORKER(Singleton *ptr);
   ~FOO_WORKER();
   void DoWork();
};

#endif


// foo_worker.cpp
// might be better with 'reference as an arg to the construct ?
FOO_WORKER::FOO_WORKER(Singleton *ptr)  : foosCommon(ptr)
{
      std::cout << "foo_worker constructor called" << std::endl;
}

FOO_WORKER::~FOO_WORKER()
{
   std::cout << "foo_worker  destructor called " << std::endl;
   // should I clean up foosCommon here ????
}

void FOO_WORKER::DoWork()
{
    foosCommon->tx_count++;
      std::cout << " tx_count = " << foosCommon->tx_count << std::endl;
}


// Now we're ready
#include "foo_worker.h"
#include "singleton_common.h"

Singleton Singleton::s;

int main ()
{
   Singleton& s = Singleton::getHandle();

   FOO_WORKER* ptrFoo = new FOO_WORKER (&s);

   for (int idx = 0; idx < 5; idx++)
   {
         ptrFoo->DoWork();
         s.Print();
   }
   delete ptrFoo;
   return 0;
}

---------------------------
The output
---------------------------
singleton_constructor called
foo_worker constructor called
 tx_count = 1
 Value of Tx Count _ Singleton_Buffer = 1
 tx_count = 2
 Value of Tx Count _ Singleton_Buffer = 2
 tx_count = 3
 Value of Tx Count _ Singleton_Buffer = 3
 tx_count = 4
 Value of Tx Count _ Singleton_Buffer = 4
 tx_count = 5
 Value of Tx Count _ Singleton_Buffer = 5
foo_worker  destructor called
Press any key to continue

How does the desing guarantee that one object of the class is created?
there's is Singleton Singleton::s;

But i could create another object. inside main
Singleton& s2 = Singleton::getHandle();   So now we have two instances.  No??

I was readign one the links where a comment was made as it relates to the static member function (Singleton class ) such that inside said function one can check if it is a first instance being created, and, if not, simply refuse the request.
How would i implement this?


0
 
LVL 15

Expert Comment

by:efn
ID: 9761349
>  // Note static class so i dont think it knows anything
> about destructing ????

Sorry, I don't understand this question.

> // might be better with 'reference as an arg to the construct ?

Maybe a little better, but I don't think it makes much difference.

More importantly, recall that I said "the class itself provides access to that one object."  That means that you no longer have to introduce it to FOO_WORKER as a parameter.

> How does the desing guarantee that one object of the class is created?

Typically, all the constructors are private.  In fact, that's the case in your implementation.

> But i could create another object. inside main
> Singleton& s2 = Singleton::getHandle();   So now we have two instances.  No??

No, that doesn't create an object.  s2 is not a new object, it's a reference to the one object that already existed.

> I was readign one the links where a comment was made as it relates to the static member function (Singleton class ) such that inside said function one can check if it is a first instance being created, and, if not, simply refuse the request.
> How would i implement this?

I'm not sure I understand your question.  Usually, as in the devx page I cited above, there is some function that hands out pointers or references to the singleton object, and that function checks whether the object has already been created.  If the object exists, the function provides access to it; if the object does not already exist, the function creates it and then provides acccess to it.  I don't know what kind of request you are talking about refusing.

--efn
0
 

Author Comment

by:forums_mp
ID: 9761391

> Sorry, I don't understand this question.
On this.  With the singleton version, i noticed the destructor was never called.  In other words I was expecting the last three lines from the output to reflect

:
Value of Tx Count _ Singleton_Buffer = 5
foo_worker  destructor called
singleton_destructor called

what i got was
Value of Tx Count _ Singleton_Buffer = 5
foo_worker  destructor called

I was investigating that but cant seem to see the reason?

In the Singleton class above the data members (for instance tx_count, rx_count) must be public for FOO_WORKER to operate on them.  Just wanting to ensure there's not some trickery I didnt forsee?


0
 
LVL 15

Expert Comment

by:efn
ID: 9761631
Most likely, the problem is not that the destructor was never called, but that the stream output didn't work in the program exit context.  I tried it with Visual C++ 6.0 and reproduced your result, but the debugger showed me that control was indeed going into the destructor, and when I put a printf call in the destructor, I did see the output.

> In the Singleton class above the data members (for instance tx_count, rx_count) must be public for FOO_WORKER to operate on them.  Just wanting to ensure there's not some trickery I didnt forsee?

Unforeseen trickery is a common problem in the practice of software engineering.  Unfortunately, I cannot guarantee that you will not encounter any in this project.  Or perhaps I didn't understand your question.

Many experts advise against having any public data members.  Using accessor functions to provide access to a class's data adds a layer of abstraction and has some advantages.  For example, you can change where the data comes from, or you can set a breakpoint in an accessor function to track accesses.  If you were to post a question here asking "Why shouldn't my class have public data members?", I expect that you would get some impassioned responses.  I personally don't have a strong belief in this guideline.

--efn
0
 

Author Comment

by:forums_mp
ID: 9762529

>> For example, you can change where the data comes from, or you can set a breakpoint in an accessor function to track accesses

I recalled reading about accessor and mutators in eckels book... could i see a example with an accessor function that'll  tx_count and rx_count?
0
 

Author Comment

by:forums_mp
ID: 9763036

efn,

I suspect I should be a bit clearer on my prior post.  What i'm after is trying to garner a feel for the case where you have

  int  tx_count1;
   int  rx_count1;
  int  tx_count2;
   int  rx_count2;
  int  tx_count3;
   int  rx_count3;

   // etc.

later
   struct MY_STRUCT 1
   {
        int aa  :  4;
        int bb  :  4;
        int cc  :  4;
        int dd  :  4;
   };

   MY_STRUCT1 my_struct;
   // more of the same

A member function to access each variable is insane.  So the  alternative  is .. welll i wont confuse the issue with my thoughts :)
0
 
LVL 15

Expert Comment

by:efn
ID: 9763926
>  could i see a example with an accessor function that'll  tx_count and rx_count?

The implementation is trivial.  There are a couple of common naming styles.

int get_txcount1();
void set_txcount1(int newValue);

int txcount1();
void txcount1(int newValue);

I have also seen

int& txcount1();

for both reading and writing, which sort of seems to defeat the purpose of having a function if it just provides access to a member.  I guess it would still be helpful for tracing accesses, though.

> A member function to access each variable is insane.

There are those who don't think so.

> So the  alternative  is ..

There are lots of alternatives.  One is just to make all the members public.  Another is to restructure the data in some conceptually meaningful way.  If you really have variables with names like count1, count2, count3, that suggests that maybe you should be using an array or a vector.  If the class is too big, maybe you should divide it up into smaller classes.  Or maybe instead of having clients access the data directly, you should have them call meaningfully named functions, e.g., instead of

foo.count1++;

you could have

foo.countAnotherFailure();

Maybe it would be appropriate for such functions to operate on more than one member.

Again, it comes down to figuring out a design where the objects and their interfaces make sense to a human reader.  It's unlikely that you can get a design with this kind of quality by starting with some C code and mechanically transforming it.

--efn
0
 

Author Comment

by:forums_mp
ID: 9767757

Perhaps a final and just so i have this in my back pocket.

>> [efn] Another is to restructure the data in some conceptually meaningful way.  If you really have variables with names like count1, count2, count3, that suggests that maybe you should be using an array or a vector

struct COUNTS
{
  int count1;
  int count2;
  int count3;
  int count4;
}
How would the increment happen inside say an increment function with an array approach, if say i want to increment count1?

void Increment( COUNTS & aa)  // or ?
{


}

Truly appreaciate your assistance.
0
 
LVL 15

Assisted Solution

by:efn
efn earned 500 total points
ID: 9767894
class CountsAndStuff
{
public:
  void incrementCount(unsigned int which);

private:
  enum NUMBER_OF_COUNTS = 4;
  int counts[NUMBER_OF_COUNTS];
};

void CountsAndStuff::incrementCount(unsigned int which)
{
  if (which < NUMBER_OF_COUNTS)
    ++counts[which];
}

CountsAndStuff cas;
cas.incrementCount(1);

Of course, you could get fancier and use names instead of magic numbers:

class CountsAndStuff
{
public:
  void incrementCount(unsigned int which);
  static const unsigned int MESSAGE_GOOD, MESSAGE_BAD, MESSAGE_LOST;

private:
  enum NUMBER_OF_COUNTS = 4;
  int counts[NUMBER_OF_COUNTS];
};

const unsigned int CountsAndStuff::MESSAGE_GOOD = 0;
const unsigned int CountsAndStuff::MESSAGE_BAD = 1;
const unsigned int CountsAndStuff::MESSAGE_LOST = 2;

CountsAndStuff cas;
cas.incrementCount(CountsAndStuff::MESSAGE_BAD);  // increments counts[1]

--efn
0
 

Author Comment

by:forums_mp
ID: 9770203
ah, but MESSAGE_BAD, MESSAGE_GOOD and MESSAGE_LOST are public data members.  Of course they're static which guarantees internal linkage but  I dont think i'll be able to access them.  Of course in class Singleton, we'll set the various counters to zero.

In other words,
class CountsAndStuff
{
public:
 CountsAndStuff ()
 {
    for (int idx=0; idx < NUMBER_OF_COUNTS, idx++)
       counts[idx] = 0;
 }
       
  void incrementCount(unsigned int which)
  {
    if (which < NUMBER_OF_COUNTS)
      ++counts[which];
  }

private:
  enum NUMBER_OF_COUNTS = 6;
  int counts[NUMBER_OF_COUNTS] = { counter1, counter2, counter3, counter4, counter5, counter6 }; // all up to 20+

  struct MSG_STRUCT1 {
   int abc;
   int def;
  }
  MSG_STRUCT1 msg_struct1;

  struct MSG_STRUCT2 {
   int aaa;
   int bbb;
  }

  MyTemplate< MSG_STRUCT2 > msg_struct;   // see below for this

};

///////////
Now FOO_WOKER needs to increment counter3.  it obviously cant so ..

Say FOO_WORKER wanted to access MSG_STRUCT1.  What sort of interface do we add for this?

Likewise what sort of interface do we add for the template.  Assume we're trying to access a function called Store?

Just trying to fully understand and at a minimum say I tried understanding - in this case - the need for 'private' data members.


FOO_WORKER will tweak them
0
 
LVL 15

Expert Comment

by:efn
ID: 9771983
> Say FOO_WORKER wanted to access MSG_STRUCT1.  What sort of interface do we add for this?

MSG_STRUCT1 is a type.  I don't know any way for a class to export a type other than to make it public.

> Likewise what sort of interface do we add for the template.  Assume we're trying to access a function called Store?

I don't know--what sort of interface would make sense conceptually for the meaning of the operation?  It doesn't make any difference that the member is a template.

--efn  
0
 

Author Comment

by:forums_mp
ID: 9774876

>>  I don't know any way for a class to export a type other than to make it public.
Sounds good to me.  truly appreaciate the help!! someday i'll be an expert :)
0

Featured Post

Free Trending Threat Insights Every Day

Enhance your security with threat intelligence from the web. Get trending threat insights on hackers, exploits, and suspicious IP addresses delivered to your inbox with our free Cyber Daily.

Join & Write a Comment

  Included as part of the C++ Standard Template Library (STL) is a collection of generic containers. Each of these containers serves a different purpose and has different pros and cons. It is often difficult to decide which container to use and …
IntroductionThis article is the second in a three part article series on the Visual Studio 2008 Debugger.  It provides tips in setting and using breakpoints. If not familiar with this debugger, you can find a basic introduction in the EE article loc…
The goal of the video will be to teach the user the concept of local variables and scope. An example of a locally defined variable will be given as well as an explanation of what scope is in C++. The local variable and concept of scope will be relat…
The viewer will be introduced to the technique of using vectors in C++. The video will cover how to define a vector, store values in the vector and retrieve data from the values stored in the vector.

759 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

20 Experts available now in Live!

Get 1:1 Help Now