Simple Class Design Q

Also, how could I implement the log file for a static method?

Yes you can create a 3rd class called CEventLog

CAccount
{

........
CEventLog objEventLog; //use objEventLog.WriteLog(..) inside the class functions since it is private.
........
public:
CAccount();
~CAccount();
GetAddressDetails()
GetAccountName()
};

CBank
{
........
CEventLog objEventLog;
........
public:
CBank();
~CBank();
GetAccount(CString& a);
AddAccount(CString& b);
DeleteAccount(CString& c);

};

CEventLog
{
CString strFilename;
public:
CEventLog();
CEventLog(CString _strFilename);
SetLogFile(CString _strFilename);
~CEventLog();
WriteLog(CString& d); //It will be good to make this function thread safe if mutiple threads
                                 //are accessing your log file.

private:
CreateLogFile(); //Call this inside the constructor CEventLog()
}

Hope I get across!

Best Regards,
DeepuAbrahamK

You may also Grab this Read Class 'CLogFile' src and extend it...

http://www.viksoe.dk/code/logfile.htm <==

-MAHESH

class CAccount
{
public:
  CAccount(CLogFile* pLogFile);

protected:
  CLogFile* m_pLogFile;
....
}

CAccount::CAccount(CLogFile* pLogFile)
{
  ASSERT(pLogFile);  //stop user just passing NULL
  m_pLogFile = pLogFile;
...
}

Similar for CBank


Create and open one file for logging, now create your CAccount and CBank objects (based on that log file) and you write to the SAME log file when you want.

To use for a static member function you need to pass the pointer to the log file directly as part of the call

eg.
static void MyFunc(CLogFile* pLogFile, ....)

And there is also the very simple technique that is usally overlooked by C++ zealots...  I use this technique in code that I know will not be in a published library...

THe logging operation is typically an application-wide service that you want to be available throughout your program... so place it at that level!  Add your CMyLogger object pointer to your CWinApp-derived object.

class CMyApp : public CWinApp
{
public:
      CMyApp();
      CMyLogger *m_pLogger;

...

Instatiate it in InitInstance,

BOOL CMyApp::InitInstance()
{
      AfxEnableControlContainer();
      CString sLogFile= gMyDir+ CSTR_LOGFILENAME;
      m_pLogger= new CMyLogger( sLogFile );
...

Then any class object can do something like:

void AnyOldObject::WriteToLog( LPCSTR sText )
{
      if ( theApp.m_pLogger && theApp.m_pLogger->fLoggingIsEnabled ) {
            gpLogger->Write( sText );
      }
}

>>>> logging operation is typically an application-wide service

if so, you should make it a singleton like the application object.

// logfile.h

class LogFile
{
     CString                  m_logfilePath;
     CRITICAL_SECTION m_cs;         // use it if in case of multi-threading
public:
     LogFile( const CString& logfilePath)
          : m_logfilePath(logfilePath) { InitializeCriticalSection(&m_cs); }
     void write(const CString& message);
};

// the one and only logfile object
extern LogFile theLogFile;


// logfile.cpp

#include "stdafx.h"
#include "logfile.h"

// the one and only logfile object
LogFile theLogFile("logfile.log");

void LogFile::write(const CString& message)
{
      EnterCriticalSection(&m_cs);
      // will throw an exception on fail
      TRY
      {
               CStdioFile logfile(m_logfilePath, CFile::modeCreate | CFile::modeNoTruncate | CFile::modeWrite | CFile::typeText);
               logfile.WriteString(message);
               logfile.Close();
           }
      CATCH (CFileException,  pfe)
      {
              LeaveCriticalSection(&m_cs);
              THROW(pfe);
              return;
       }
       END_CATCH
       LeaveCriticalSection(&m_cs);
 }

// anyothersource.cpp

#include "stdafx.cpp"
#include "logfile.h"


   ....
   theLogFile.write("Hello Logfile\n");

Regards, Alex

juz a bit of correction to itsmeandnobodyelse,
the above codes does not really follows the design pattern singleton
the characteristic for singleton is that there can and will only be 1 instance of the class
the correct implementation for singleton follows the following pattern

public class Singleton {
    private:
        static Singleton *instance;
        Singleton() //note the use of private constructor
        {
             //do your initialization of your class here
        }
    public:
        static Singleton getInstance()
        {
            if (instance==NULL)
            {
                instance=new Singleton();
            }
            return instance;
        }
}

Singleton::instance=NULL;

when using the singleton class, you will call the getInstance function for the singleton class, ie
Singleton::getInstance()
however, you will need to call delete the instance of singleton manually to prevent memory leak

Thanks,
What would the structure of the Singleton be for a log file? Should the getInstance() method be subsituted with a createLogFile(CString cstrFileName) method???

>> DanRollins

>>And there is also the very simple technique that is usally overlooked by C++ zealots...  I use this technique in code that I know will not be in a published library...

>>THe logging operation is typically an application-wide service that you want to be available throughout your program... so place it at that level!  Add your CMyLogger object pointer to your CWinApp-derived object.

I am having difficulty implementing this method, specifically accessing the m_pLogger from another class.  

When do the following:

if ( theApp.m_pLogger && theApp.m_pLogger->fLoggingIsEnabled )

I get an error stating

 error C2065: 'theApp' : undeclared identifier
 error C2227: left of '.m_plogger' must point to class/struct/union

This also occurs even when I declare (and try to access it in a different class) a simple test variable in the CMyApp header, say "int x".

You need the extern keyword.

eg in your .cpp file.
extern int DefinedElsewhere;    //similar for the log file



ps.
Don't forget one of the reasons for using C++ is to allow one to only access variables where they are required (member vars of classes, and pointers to them).  Using an application global variable this way goes against that methodology.  

I would advise passing the pointer to the log file directly via functions.  It can cope easily with one or multiple log files.

>>>> error C2065: 'theApp' : undeclared identifier

normally 'theApp' is only defined properly - or better as expected by Dan and me - if the MFC application was using the MFC framework and was created by using the Application Wizard and is an SDI or MDI application. Then, your CWinApp derived application class has a (global) extern definition of the single instance of that class at end of the application header:

//myapp.h


....

class CMyApp : public CWinApp
{
  ...
};

// the one and only application instance

extern CMyApp theApp;

....

// myapp.cpp

...

// here 'the one and only application instance' was defined
CMyApp theApp;

...


In all other cpp you have access to the 'theApp' global variable by including myapp.h or declaring it as extern again:

   extern CMyApp theApp;

If your application header has no extern declaration but theApp was defined in the application cpp, you simply could add the extern declaration after class definition.


Note, as Andy explained global variables is a C technique (which there are many in MFC). In C++ you could realize singletons by using static member functions as showed above.  Note, that doesn't apply to 'theApp' cause it was used by MFC sources.

For other singletons you could/should add them as static members in the class itself:

class MySingleton
{
  ...

public:
     static CMySingleton m_mySingleton;
};

to the class definition and define it in the cpp

// mysingleton.cpp

    CMySingleton CMySingleton::m_mySingleton;

You would need to specifiy the scope CMySingleton:: when using it but it is a good C++ way to replace global instances.

Regards, Alex

You can also access the CWinApp-derived object as follows:

in any module:

   #include "MyApp.H"       // the wizard probably already added this for you

    ....

    CMyApp* pMyApp = (CMyApp*) AfxGetApp();

    int y= pMyApp->x;
    pMyApp->LogMsg( "hi!");

itsmeandnobodyelse

Regarding the Singleton implementation:
Where should the Open(CString filename, CString filepath) method be placed - in the constructor?
Why would stop one doing the following:

LogFile logfile("c:/directory/");

logfile.write("Bla bla bla");

sorry, that should read what should stop one doing the following:

In my implementations of a logfile I open and close the logfile in the write function. That way I can open the logfile with an editor while the program is still running (otherwise you might get sharing violations). Furthermore, if writing to the logfile in a multi-threaded program you need to make synchronization or the call might hang up. Closing the file with each log additionally flushes the io buffer, so your logfile is always up-to-date.

>>>> what should stop one doing the following:
>>>> LogFile logfile("c:/directory/");

You could try to create (or open and append) the file given in the constructor. Maybe you want to write a start message to the logfile (timestamp) as well. If file creation fails you could call assert() to throw a debug assertion or you might throw a C++ exception if the logfile can be wrong in release mode also. If you don't like exceptions you could add a bool member 'isvalid' to your Logfile class and set it to false if file creation fails. Then, after Logfile creation you could check validity:

    LogFile logfile("c:/directory/");
    if (!logFile.isValid())
    {
          cout << "logfile " << logFile.getLogfilePath() << " can't be opened." << endl;
          return 2;
    }
   
Regards, Alex