#177 Fix & document afx header includes

closed-rejected
Perry
None
5
2003-07-18
2003-06-29
Perry
No

Add either forward class declarations, or guarded
includes of afx headers (with comment as to purpose)
where needed, so that individual files are
self-documenting about their dependencies, and don't
depend on their dependencies being handled in stdafx.h.

But, still include three of the afx headers in stdafx.h
for speed of precompiled headers.

Discussion

  • Perry
    Perry
    2003-06-29

    Altered (& original) files affected

     
  • ganier
    ganier
    2003-07-03

    Logged In: YES
    user_id=804270

    I understand the patch as having two goals :
    * to help documenting the dependencies
    * to clean stdafx.h from some dependency ( <afxcmn.h> in
    fact)
    It seems me some comments lines about dependencies are
    welcome, but we should not use class declarations or includes
    for documenting purposes ! We can just write an introduction
    sentence and comment the includes lines you added.

     
  • Perry
    Perry
    2003-07-03

    Logged In: YES
    user_id=60964

    Do you mean instead of this (which is in my patch):

    #include <afxtempl.h> // MFC C++ template collection
    classes

    doing this (an introductory comment):

    // MFC C++ template collection classes
    #include <afxtempl.h>

    ?

    If so, I can do that. I might myself rather prefer to
    discard the comment than to spend an extra line for it -- if
    you would go for blank (uncommented) lines, I could revise
    it to be so, eg,

    #include <afxtempl.h>

    ?

     
  • ganier
    ganier
    2003-07-03

    Logged In: YES
    user_id=804270

    No I really mean commenting the include as:

    // Includes files from Stdafx.h which are necessary :
    // <afxtempl.h>

     
  • Perry
    Perry
    2003-07-03

    Logged In: YES
    user_id=60964

    Oh. I guess I don't understand your explanatory sentence
    "Includes files from Stdafx.h which are necessary".

    Those files don't come from Stdafx.h (which is a file in our
    project); they come from the Visual C++ MFC include directory.

     
  • Perry
    Perry
    2003-07-03

    Logged In: YES
    user_id=60964

    The idea of documenting classes is an interesting point. I'm
    actually accustomed to documenting classes in their headers,
    but I'm interested in any system you propose -- I think this
    project doesn't have very standard commenting documentation.

    We could consider some system that can generate html files,
    such as doxygen (which I have used), or some of the similar
    ones (which I've not used) ?

     
  • ganier
    ganier
    2003-07-03

    Logged In: YES
    user_id=804270

    The explanatory sentence is not clear, so I guess I need your
    help to find the words. What I mean is <afxtempl.h> is
    included in Stdafx.h, and the .cpp file includes Stdafx.h, so
    we don't need to include it in the cpp file, and even we
    should not. But add a comment to tell we herit <afxtempl.h>
    from Stdafx.h is a good idea.

    I have used a system to generate html but it was long ago.
    Anyway it would bring a great help. I don't know if the
    generated html has a place on the SourceForge site, as we
    can generate the html doc on our own computer. Except if
    somebody wants to add info to the html, but personnally I
    don't feel it is a priority.

     
  • Perry
    Perry
    2003-07-03

    Logged In: YES
    user_id=60964

    Oh, I misunderstood. Ok, maybe,
    // Assumes the following files are included by Stdafx.h:
    // afxtempl.h

    Although, I prefer to explicitly include them in the cpp,
    from when I have gotten tired of trying to reuse cpp files
    and having to figure out what headers they include. If they
    are included, then I don't have to figure it out.

    Actually all the MFC includes I've seen are protected
    against multiple inclusion, so it is safe to include them
    multiple times. But, you can be even more efficient, and
    include like so:

    #ifndef __AFXTEMPL_H__
    #include <afxtempl.h> // MFC C++ template collection
    classes
    #endif // __AFXTEMPL_H__

    This way, if the Stdafx.h already includes afxtempl.h, then
    the parser doesn't even read it again (because that symbol
    is defined).

    I guess I've adopted this strategy from when I've reused
    entire source files -- I much preferred that if two projects
    use the same source files, they use them *exactly* the same.

    Perhaps it doesn't matter in this project, as it doesn't
    share any (C++) source files with anything else.

    I'll post in the developers forum about documentation :)

     
  • Kimmo Varis
    Kimmo Varis
    2003-07-11

    Logged In: YES
    user_id=631874

    Some MFC guides are proposing to add most #include lines
    (even non-MFC includes) to stdafx.h and then including
    stdafx.h everywhere. Reasons given are compile speed and
    simpler #include management. I personally don't agree that
    non-MFC includes -part. But it's interesting point. WinMerge
    has problems with CrystalEditor #includes.

    CrystalEditor include lines are just a mess. Try to add new
    one or change something. I tried to add #include "files.h"
    to ccrystaltextbuffer.h to remove all file-handling code
    (eol-reading etc) from it. I tried it for three hours or so
    and give up.

    So I definitely agree we need some cleanup for #includes.

    At least some big projects (e.g. Mozilla) prefer using
    forward class declarations. I'm not sure about MFC includes.
    Maybe it's easiest solution to add them to only stdafx.h?
    It's easier when adding new files, just include stdafx.h and
    you get all you need. Yes, per file inclusion may be little
    more efficient, but it can be pain when you have to change
    or add new includes.

     
  • Perry
    Perry
    2003-07-11

    Logged In: YES
    user_id=60964

    re: forward class declarations

    I *always* prefer forward class declarations. But you can't
    do them for templates (at least I can't; sometimes it is
    possible if you understand typename stuff, maybe, but I
    doubt Visual C++ can handle that -- its template support is
    very out-of-date), and you can't do them for embedded members.

    Anyway, shall I just mark this patch rejected, and we'll
    abandon this ?

     
  • Kimmo Varis
    Kimmo Varis
    2003-07-11

    Logged In: YES
    user_id=631874

    You know this area better than me so it's your call. If I
    remember correctly this patch added couple of forward
    declarations? Maybe those are worth checking in at least?

     
  • Perry
    Perry
    2003-07-18

    Logged In: YES
    user_id=60964

    I've lost interest in this, and am marking it as
    Closed/Rejected to help clean up the patch list :)

     
  • Perry
    Perry
    2003-07-18

    • assigned_to: nobody --> puddle
    • status: open --> closed-rejected