views:

72

answers:

1

Edited by OP.
My program is in need of a lot of cleanup and restructuring.

In another post I asked about leaving the MFC DocView framework and going to the WinProc & Message Loop way (what is that called for short?). Well at present I am thinking that I should clean up what I have in Doc View and perhaps later convert to non-MFC it that even makes sense. My Document class currently has almost nothing useful in it.

I think a place to start is the InitInstance() function (posted below).
In this part:

POSITION pos=pDocTemplate->GetFirstDocPosition();
CLCWDoc *pDoc=(CLCWDoc *)pDocTemplate->GetNextDoc(pos);
ASSERT_VALID(pDoc);
POSITION vpos=pDoc->GetFirstViewPosition();
CChildView *pCV=(CChildView *)pDoc->GetNextView(vpos);

This seem strange to me. I only have one doc and one view. I feel like I am going about it backwards with GetNextDoc() and GetNextView(). To try to use a silly analogy; it's like I have a book in my hand but I have to look up in it's index to find out what page the Title of the book is on. I'm tired of feeling embarrassed about my code. I either need correction or reassurance, or both. :)

Also, all the miscellaneous items are in no particular order. I would like to rearrange them into an order that may be more standard, structured or straightforward.

ALL suggestions welcome!

BOOL CLCWApp::InitInstance()
{
 InitCommonControls();
 if(!AfxOleInit())
  return FALSE;

    // Initialize the Toolbar dll. (Toolbar code by Nikolay Denisov.)
 InitGuiLibDLL(); // NOTE: insert GuiLib.dll into the resource chain

 SetRegistryKey(_T("Real Name Removed"));

 // Register document templates
 CSingleDocTemplate* pDocTemplate;
 pDocTemplate = new CSingleDocTemplate(
  IDR_MAINFRAME,
  RUNTIME_CLASS(CLCWDoc),
  RUNTIME_CLASS(CMainFrame),
  RUNTIME_CLASS(CChildView));
 AddDocTemplate(pDocTemplate);

 // Parse command line for standard shell commands, DDE, file open
 CCmdLineInfo cmdInfo;
 ParseCommandLine(cmdInfo);

 // Dispatch commands specified on the command line
 // The window frame appears on the screen in here.
 if (!ProcessShellCommand(cmdInfo))
 {
  AfxMessageBox("Failure processing Command Line");
  return FALSE;
 }

 POSITION pos=pDocTemplate->GetFirstDocPosition();
 CLCWDoc *pDoc=(CLCWDoc *)pDocTemplate->GetNextDoc(pos);
 ASSERT_VALID(pDoc);
 POSITION vpos=pDoc->GetFirstViewPosition();
 CChildView *pCV=(CChildView *)pDoc->GetNextView(vpos);
 if(!cmdInfo.m_Fn1.IsEmpty() && !cmdInfo.m_Fn2.IsEmpty())
 {
  pCV->OpenF1(cmdInfo.m_Fn1);
  pCV->OpenF2(cmdInfo.m_Fn2);
  pCV->DoCompare(); // Sends a paint message when complete
 }
 // enable file manager drag/drop and DDE Execute open
 m_pMainWnd->DragAcceptFiles(TRUE);

 m_pMainWnd->ShowWindow(SW_SHOWNORMAL);
 m_pMainWnd->UpdateWindow(); // paints the window background

 pCV->bDoSize=true; //Prevent a dozen useless size calculations

 return TRUE;
}

Thanks

+4  A: 

Hard to give you good recommendations without knowing what your program shall do. I have only a few general remarks:

  • Your InitInstance does not look very messed up for me. It's pretty much standard with a bit of custom code in it.
  • Also the ugly construction to retrieve the first view from the application class (the chain GetDocTemplate -> GetDoc -> GetView) is standard to my knowledge. I actually don't know another way. You might think about moving it into a separate method like CChildView* CLCWApp::GetFirstView() but well, that's only cosmetic as long as you need it only at one place.

What you are doing and which data you are placing in your Document class and in your View class(es) is more a semantic question if you only have one view. (You have only one document anyway because it's an SDI application.). From a technical viewpoint often both is possible. But to be open for (perhaps) later extensions to more than one view and to follow the standard pattern of a doc/view architecture there are a few rules of thumb:

  • Data which exist and have a meaning independent of the way to present and view them (a document file, a database handle, etc.) belong to the document class. I don't know what your pCV->OpenF1(cmdInfo.m_Fn1) ... and so on does but if it's something like a file or filename or a parameter to be used to access data in any way OpenF1 might be better a method of the document class.
  • Methods which do any kind of data processing or modification of your underlying data belong to the document class as well
  • Data and methods which are only needed for a specific way to display a document belong to a view class (for instance a selected font, colours, etc.)
  • On the other side: If you have a fixed number of views which open with the document it might not be wrong to put view specific data into the document, especially if you want to make those view parameters persistent. An example would be a file with some statistical data - your document - and a splitter frame with two views: one displays the data as a grid table and the other as a pie chart. The table has "view data" describing the order of and width of columns, the pie chart has data to configure the colours of the pie pieces and the legend location, for instance. If you want to make sure that the user gets the last view configuration displayed when he opens the document file you have to store these view parameters somewhere. It wouldn't be wrong or bad design in my opinion to store those parameters in the document too, to store and retrieve them from any permanent storage, even if you need them only in the view classes.
  • If your application allows to open an unlimited number of views for a document dynamically and those views are only temporary as long as the application runs, storing all view configuration parameters directly in the view classes seems more natural to me. Otherwise in the document you would need to manage any kind of dynamic data structure and establish a relationship between a View and an entry in this data structure (an index in an array, or a key in a map, etc.)
  • If you are in doubt whether to place any data in the document or view class I'd prefer the document because you always have the easy GetDocument() accessor in the View class to retrieve members or call methods of the Doc. To fetch data from the View into the Document requires to iterate through the list of views. (Remember: Doc-View is a 1-n relationship, even in a SDI application.)

Just a few cents.

Slauma
Thank you very much. It will take a while to process the info. This definitely helps to sort out my conflicting ideas of where I should go with it. Yes, pCV->OpenF1(cmdInfo.m_Fn1) is opening a file. I shall try to start by moving that stuff into the Doc. Thank you for your kind reassurance. Very helpful.
Harvey
No need to be embarrassed by the auto generated boilerplate code.
Mark Ransom
@Mark, Thank you.
Harvey
Harvey, if your document really represents more or less a file it is worth to take a look at the various methods in CDocument which are called by the MFC framework automatically, like OnNewDocument, OnOpenDocument, OnCloseDocument, IsModified, SaveModified, etc. CDocument is designed to deal with files this way. (Consider NotePad as a simple Doc/View SDI application with one document and one view par excellence. The menu items in the File menu ("New", "Open", "Close", "Save", etc. are tightly related to those CDocument methods).
Slauma
@Slauma, Thank you for commenting. Actually it is a file compare program, so the files are not modified by my program (as yet). And even the compare (as a document) is only written to a file for debugging purposes (so far). So there is nothing "persistent" in "a document" (yet).
Harvey