Request for Code Review

Discussion in 'C++' started by Ben Hanson, Jul 2, 2004.

  1. Ben Hanson

    Ben Hanson Guest

    I have created an open source Notepad program for Windows in C++ that allows
    search and replace using regular expressions (and a few other extras). It
    is located at http://www.codeproject.com/cpp/notepadre.asp

    I'm trying to use best practice in my C++ programming and would appreciate
    any advice anyone can give. As code is far more than just a one page
    sample, just a review of one of the source files (or even just a function or
    two) is equally welcome!

    If you reply, could you also mail me at benjamin dot hanson at swx dot ch

    Thanks!


    --
    ~ Samba, more than a low cost File and Printer server ~

    -- Let us OpenSource --


    -----= Posted via Newsfeeds.Com, Uncensored Usenet News =-----
    http://www.newsfeeds.com - The #1 Newsgroup Service in the World!
    -----== Over 100,000 Newsgroups - 19 Different Servers! =-----
     
    Ben Hanson, Jul 2, 2004
    #1
    1. Advertising

  2. * Ben Hanson:
    > I have created an open source Notepad program for Windows in C++ that allows
    > search and replace using regular expressions (and a few other extras). It
    > is located at http://www.codeproject.com/cpp/notepadre.asp
    >
    > I'm trying to use best practice in my C++ programming and would appreciate
    > any advice anyone can give. As code is far more than just a one page
    > sample, just a review of one of the source files (or even just a function or
    > two) is equally welcome!


    I'm currently a bit depressed because some folks who promised to
    review something for me haven't even replied that they've not found
    the time (or whatever). So I'll take a look at this, just to prove
    to myself that it's no big hassle to review something. Then I can
    go from depressed to perhaps a bit angry.


    > If you reply, could you also mail me at benjamin dot hanson at swx dot ch


    Nope (see the FAQ).

    Okay, first problem: in order to download the code one must provide a
    username and password. I can't for the life of me remember what I chose
    last time at that ****ing site. But there is a quick-register option
    where I only have to supply an e-mail address. OK! But alas, "The
    email you supplied has already been registered".

    Now checking the mail to see if they've sent me my old username etc...

    Nope. One idiot-mail from someone quoting a usenet posting of mine (in
    sci.physics.researc) and adding nothing more; one attempted virus
    (automatically removed); one spam apparently from ,
    "Winning Notice", yeah right.

    Okay, found option to have the site mail me my old username & password.

    Now checking mail again...

    Nope, nothing.

    Now checking mail again...

    Nope, nothing.

    Now checking mail again...

    Nope, nothing.

    Now checking mail again...

    Nope, nothing.

    Now checking mail again...

    Nope, nothing.

    Well I ain't gonna create a new e-mail account just to download your
    complete code, so I'll have to review just what you display on the page.

    The first thing you state about the internals is that "Notepad RE is
    CEditView based". Now what is "CEditView"? Since this is Windows I
    fire up my MSDN Library. Yeah, right, there is a CEditView class in
    MFC. But, MFC? Why don't you use WTL?

    * Recommendation: state first of all that this is an MFC app.

    Some folks might say that has nothing to do with C++. But it has.
    MFC is C in C++ disguise. WTL is at least template-based, although
    it's Microsoft style with e.g. unsafe construction and all that.

    The first bit of actual code displayed is

    void CFindReplaceDlg::OnCheckMatchCase ()
    {
    m_pParentWnd->PostMessage (WM_NOTIFY_FIND_REPLACE, eMatchCase,
    m_CheckMatchCase.GetCheck ());
    }

    Now this is using a Windows API service in order to update (?) the
    dialog owner window's state -- I think. But using that service
    (message posting) seems to be unnecessary. And it introduces type
    unsafety.

    Better just call a method on the parent window (or a more restricted
    interface) directly.

    Next, we have

    BOOL CFindReplaceDlg::OnInitDialog()
    {
    CDialog::OnInitDialog ();

    m_CheckWholeWordOnly.SubclassDlgItem (IDC_CHECK_WHOLE_WORD, this);

    if (m_bInitialRegEx)
    {
    m_CheckWholeWordOnly.EnableWindow (FALSE);
    }
    else
    {
    m_CheckWholeWordOnly.SetCheck (m_bInitialWholeWordOnly);
    }

    m_CheckMatchCase.SubclassDlgItem (IDC_CHECK_MATCH_CASE, this);
    m_CheckMatchCase.SetCheck (m_bInitialMatchCase);
    m_CheckRegEx.SubclassDlgItem (IDC_CHECK_REGEX, this);
    m_CheckRegEx.SetCheck (m_bInitialRegEx);

    if (m_IDD == IDD_FIND)
    {
    m_RadioUp.SubclassDlgItem (IDC_RADIO_UP, this);
    m_RadioDown.SubclassDlgItem (IDC_RADIO_DOWN, this);
    m_RadioDown.SetCheck (TRUE);
    if (m_bInitialRegEx) m_RadioUp.EnableWindow (FALSE);
    m_CheckCloseOnMatch.SubclassDlgItem (IDC_CHECK_CLOSE_ON_MATCH,
    this);
    }
    else
    {
    m_EditReplaceText.SubclassDlgItem (IDC_EDIT_REPLACE, this);
    m_CheckReplaceAllLikeNotepad.SubclassDlgItem
    (IDC_CHECK_LIKE_NOTEPAD,
    this);
    }

    m_EditFindText.SubclassDlgItem (IDC_EDIT_FIND, this);
    m_EditFindText.SetWindowText (m_strInitialFind);
    return TRUE; // return TRUE unless you set the focus to a control
    // EXCEPTION: OCX Property Pages should return FALSE
    }

    The first thing I notice is inconsistent indentation size; previously it
    was 4, now it's 2.

    * Recommendation: use consistent indentation size.

    The OnInitDialog mess is forced upon you by MFC; nothing much one can do
    about that in an MFC-based application, but otherwise, use constructors
    for construction.

    * Recommendation: don't use MFC, it's not type-safe.

    Next thing I notice is that there seems to be no failure checking. Not
    even asserts. Perhaps that's because all those methods invoked are nice
    enough to throw exceptions if they fail, but remembering what MFC was
    like I rather doubt it.

    * Recommendation: do check for failures.

    Third thing I notice is that m_IDD == IDD_FIND is repeated later on in
    other code.

    This suggest that you should really have two different classes, perhaps
    derived from a class that encapsulates the _common_ things.

    * Recommendation: do use inheritance to factor out common things,
    and do use different classes for objects with different
    functionality.

    New function, CNotepadreDoc::OnNotifyFindReplace. I'll not repeat the
    code here, but it's one long if-then-elseif-elsif-elsif just checking
    a variable's value against a set of possible (apparently) constant
    values. Now that's perfectly OK technically, but it would be more clear
    (especially that no other kind of condition is present) with a 'switch':

    * Recommendation: do use 'switch' for 'switch' constructions.

    New function, CNotepadreDoc::OnOpenDocument.

    Here you have some decidedly non-C++ exception handling based on _old_
    MFC macros.

    * Recommendation: do use modern C++ exception handling. Even
    Microsoft says so in their documentation of MFC!

    * Check out the ScopeGuard header (Google), use ScopeGuard for the
    cleanup things you here use try-catch for, and in general (but not
    in this specific case) use RAII (Google) rather than try-catch.

    Now that's that, I think I've done enough to change my emotional state a
    bit in the right direction, and in a short time!

    Cheers,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Jul 2, 2004
    #2
    1. Advertising

  3. Ben Hanson

    Ben Hanson Guest

    Thanks for the prompt reply!

    The ScopeGuard tip alone deserves that I return the favour. Can you post a
    URL or zip?

    Cheers

    Ben


    --
    ~ Samba, more than a low cost File and Printer server ~

    -- Let us OpenSource --


    -----= Posted via Newsfeeds.Com, Uncensored Usenet News =-----
    http://www.newsfeeds.com - The #1 Newsgroup Service in the World!
    -----== Over 100,000 Newsgroups - 19 Different Servers! =-----
     
    Ben Hanson, Jul 2, 2004
    #3
  4. * "Ben Hanson" <benjamin dot hanson at swx dot ch>:
    > Thanks for the prompt reply!
    >
    > The ScopeGuard tip alone deserves that I return the favour. Can you post a
    > URL or zip?


    Using Google the first real reference I found was the Norwegian
    [no.it.programmering.c++] FAQ (which I contributed to), at
    <url: http://utvikling.com/cppfaq/04/04/02/>; see point D.

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Jul 2, 2004
    #4
  5. Off Topic: Request for Code Review

    Ben Hanson wrote:

    > I have created an open source Notepad program for Windows in C++


    Sorry.
    Your code does *not* comply with the ANSI/ISO C++ standard.
    Try posting your request to an appropriate Microsoft Windows newsgroup.
     
    E. Robert Tisdale, Jul 2, 2004
    #5
  6. Ben Hanson wrote:

    > I have created an open source Notepad program for Windows in C++ that
    > allows
    > search and replace using regular expressions (and a few other extras). It
    > is located at http://www.codeproject.com/cpp/notepadre.asp
    >
    > I'm trying to use best practice in my C++ programming and would appreciate
    > any advice anyone can give. As code is far more than just a one page
    > sample, just a review of one of the source files (or even just a function
    > or two) is equally welcome!
    >
    > If you reply, could you also mail me at benjamin dot hanson at swx dot ch
    >
    > Thanks!
    >
    >


    This may be a minority view, but I prefer to put a lot more comments in my
    code.
    --
    Chris Gordon-Smith
    London
    Homepage: http://graffiti.virgin.net/c.gordon-smith/
    Email Address: Please see my Home Page
     
    Chris Gordon-Smith, Jul 2, 2004
    #6
  7. Re: Off Topic: Request for Code Review

    E. Robert Tisdale wrote:

    > Ben Hanson wrote:
    >
    >> I have created an open source Notepad program for Windows in C++

    >
    > Sorry.
    > Your code does *not* comply with the ANSI/ISO C++ standard.
    > Try posting your request to an appropriate Microsoft Windows newsgroup.


    Isn't this a rather narrow point of view? It seems to suggest that this
    newsgroup is for an elite cognoscenti, and that anyone else should go away.

    Perhaps that is not what you intended, but that is the impression that comes
    across.
    --
    Chris Gordon-Smith
    London
    Homepage: http://graffiti.virgin.net/c.gordon-smith/
    Email Address: Please see my Home Page
     
    Chris Gordon-Smith, Jul 2, 2004
    #7
  8. Re: Off Topic: Request for Code Review

    Chris Gordon-Smith wrote:

    > E. Robert Tisdale wrote:
    >
    >>Ben Hanson wrote:
    >>
    >>>I have created an open source Notepad program for Windows in C++

    >>
    >>Sorry.
    >>Your code does *not* comply with the ANSI/ISO C++ standard.
    >>Try posting your request to an appropriate Microsoft Windows newsgroup.

    >
    > Isn't this a rather narrow point of view?
    > It seems to suggest that this newsgroup is for an elite cognoscenti,
    > and that anyone else should go away.


    The opposite is true.
    Ben Hansen's implementation is restricted to "an elite cognoscenti"
    consisting of Microsoft Windows C++ programmers.
    No one else can actually compile his code much less comment on it.

    Ben should submit his request for a code review to a forum
    consisting of "an elite cognoscenti"
    that specializes in Microsoft Windows C++.

    > Perhaps that is not what you intended,
    > but that is the impression that comes across.


    Subscribers to the comp.lang.c++ newsgroup have wider interests
    than Microsoft Windows C++.
    Perhaps you don't mean to be inconsiderate or disrespectful of them
    but that's the impression that comes across.
     
    E. Robert Tisdale, Jul 3, 2004
    #8
  9. Ben Hanson

    Mike Wahler Guest

    Re: Off Topic: Request for Code Review

    "Chris Gordon-Smith" <> wrote in message
    news:...
    > E. Robert Tisdale wrote:
    >
    > > Ben Hanson wrote:
    > >
    > >> I have created an open source Notepad program for Windows in C++

    > >
    > > Sorry.
    > > Your code does *not* comply with the ANSI/ISO C++ standard.
    > > Try posting your request to an appropriate Microsoft Windows newsgroup.

    >
    > Isn't this a rather narrow point of view?


    By necessity, the topic of this group is as 'narrow' as the
    standard C language. If it were otherwise, there are so
    many platform-specific and third-party libraries and interfaces,
    etc. that it would be nigh impossible for those interested only
    in the language (the topic here, after all) to locate posts
    about it.

    > It seems to suggest that this
    > newsgroup is for an elite cognoscenti,


    It's for those who discuss ISO standard C, regardless of their
    knowledge and skill levels with it. If you consdier that group
    to be 'elite', so be it.

    >and that anyone else should go away.


    Anyone is welcome to read and post topical articles here.

    > Perhaps that is not what you intended, but that is the impression that

    comes
    > across.


    Robert appears to frequently annoy many folks here, but imo
    this time he's right. Perhaps one might find his post lacks
    tact.

    But think about it. In order to intelligently discuss Ben's
    code, one need to understand and discuss the Windows API,
    which has *nothing* to do with ISO C. Ben would get much
    better and high quality responses from Windows experts by
    posting to a Windows group, e.g. comp.os.ms-windows.programmer.32
    (I read that group fairly frequently, and there are a few imo quite
    impressive 'gurus' that answer questions there.

    If I want high quality information, advice, or assistance with
    ISO C, I post here. Here is where the C experts discuss C
    (not Windows or anything else).


    -Mike
     
    Mike Wahler, Jul 3, 2004
    #9
  10. Ben Hanson

    Mike Wahler Guest

    Re: (corr) Off Topic: Request for Code Review

    "Mike Wahler" <> wrote in message news:dpmFc.2982>

    .... some stuff.

    Change all occurrences of C to C++.
    Sorry about that.

    -Mike
     
    Mike Wahler, Jul 3, 2004
    #10
  11. Re: Off Topic: Request for Code Review

    * Mike Wahler:
    > "Chris Gordon-Smith" <> wrote in message
    > news:...
    > > E. Robert Tisdale wrote:
    > >
    > > > Ben Hanson wrote:
    > > >
    > > >> I have created an open source Notepad program for Windows in C++
    > > >
    > > > Sorry.
    > > > Your code does *not* comply with the ANSI/ISO C++ standard.
    > > > Try posting your request to an appropriate Microsoft Windows newsgroup.

    > >
    > > Isn't this a rather narrow point of view?

    >
    > By necessity, the topic of this group is as 'narrow' as the
    > standard C language.


    Well, well, well.

    Ben Hanson asked for review of the C++ aspects, not Windows programming,
    not C programming.




    > Robert appears to frequently annoy many folks here, but imo
    > this time he's right. Perhaps one might find his post lacks
    > tact.


    I think Robert-discussions are off-topic here.



    > But think about it. In order to intelligently discuss Ben's
    > code, one need to understand and discuss the Windows API,


    Not at all.

    By that argument no real code could be discussed in this group.



    > Ben would get much
    > better and high quality responses from Windows experts by
    > posting to a Windows group, e.g. comp.os.ms-windows.programmer.32
    > (I read that group fairly frequently, and there are a few imo quite
    > impressive 'gurus' that answer questions there.


    Bah. I'm better than most of those top-posting idiots... ;-) But
    in this group we should discuss C++, not system XYZ programming.



    > If I want high quality information, advice, or assistance with
    > ISO C, I post here. Here is where the C experts discuss C
    > (not Windows or anything else).


    Again, Mike, "here" is [comp.lang.c++], no cross-posting.


    Cheers,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Jul 3, 2004
    #11
  12. Ben Hanson

    Ben Hanson Guest

    Crossed wires there!

    I found http://www.cuj.com/documents/s=8000/cujcexp1812alexandr/alexandr.htm
    yesterday and as I say just this tip alone deserves I return the favour and
    review your code.

    Do you have a URL or zip for _your code_, so I can review it?

    Thanks

    Ben


    --
    ~ Samba, more than a low cost File and Printer server ~

    -- Let us OpenSource --


    -----= Posted via Newsfeeds.Com, Uncensored Usenet News =-----
    http://www.newsfeeds.com - The #1 Newsgroup Service in the World!
    -----== Over 100,000 Newsgroups - 19 Different Servers! =-----
     
    Ben Hanson, Jul 3, 2004
    #12
  13. Ben Hanson

    Ben Hanson Guest

    Re: Off Topic: Request for Code Review

    Hi there,

    I'd like to present my own view.

    Yes, I deveop on Windows and yes, all the GUI parts of the code are Windows
    specific. However, C++ interests me as a language and the cross platform
    possibilities have always interested me, even though I've not had the
    occaision to port my code to Linux etc.

    All the points Alf raised are interesting and he really caught my interest
    with his comment on try-catch and ScopeGuard. This is a perfect example of
    how the old Microsoft style can be replaced with a modern C++ style. This
    is of great interest to me.

    Originally the code used a C++ wrapper to Henry Spencer's regular expression
    library and some C code i wrote to do string matching for non-regex
    searching. The search routine was very MS like and not particularly
    readable. Since then I have switched to the boost library for regex support
    and std::search from the C hack I was using previously. If nothing else
    these two techniques are platform independant.

    I've contacted both Bjarne Stroustrup and John Maddock (boost regex) and
    they've both given me great advice more than once. I can tell you that
    Bjarne was very concerned when I was having problems with
    std::reverse_iterator in Visual C++ 2002. Cross platform compatibility is
    one of his highest priorities and he tests the Microsoft compilers for
    compliance as well as all the GNU stuff etc. If it's good enough for him...

    And finally, I've worked with Microsoft zealots that bitched and moaned
    about C++ and complained that the C++ Standard Library was too hard, too
    buggy, too slow, pointless etc. (Gosh it wasn't written by MS, so it must
    be rubbish, right?). You won't be surprised to learn those guys switched to
    VB, and good riddance. I imagine a new generation will be delighted to
    switch to C# - I'm not one of those people either. I won't even get into
    Java, or we'll be here all day/night!

    Forgive my choice of platform, but I really wanted comments on my C++, not
    my choice of Windows API calls... It's not fun to be "shot by both sides",
    both by the frothing MS zealots on the one hand and the GNU/Linux/ANSI C++
    (please delete as appropriate) crowd on the other. Apparantly, even
    releasing open source just isn't good enough for some people if it's not for
    their approved platform.

    Well I guess by now you realise how I feel about this issue..!

    Kind Regards

    Ben Hanson


    --
    ~ Samba, more than a low cost File and Printer server ~

    -- Let us OpenSource --


    -----= Posted via Newsfeeds.Com, Uncensored Usenet News =-----
    http://www.newsfeeds.com - The #1 Newsgroup Service in the World!
    -----== Over 100,000 Newsgroups - 19 Different Servers! =-----
     
    Ben Hanson, Jul 3, 2004
    #13
  14. Ben Hanson

    JKop Guest

    Re: Off Topic: Request for Code Review

    I like to do the following:

    namespace Windows { #include <windows.h> }

    that's keeps all of their crap away from your stuff, unfortunately though,
    the macros still get through.


    -JKop
     
    JKop, Jul 3, 2004
    #14
  15. * "Ben Hanson":
    >
    > Do you have a URL or zip for _your code_, so I can review it?


    Thanks, mailed it (it's not for public consumption, at least not yet).

    Cheers,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Jul 3, 2004
    #15
  16. Re: Off Topic: Request for Code Review

    * JKop:
    >
    > I like to do the following:
    >
    > namespace Windows { #include <windows.h> }
    >
    > that's keeps all of their crap away from your stuff, unfortunately though,
    > the macros still get through.


    Unfortunately you'll have to add to your work-around definitions every
    time you use something new from the API (relative to earlier work).

    Yes it can be done.

    No, in my experience it's not really worth it; it's a lot of work.

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Jul 3, 2004
    #16
  17. Ben Hanson

    JKop Guest

    Re: Off Topic: Request for Code Review

    Alf P. Steinbach posted:

    > * JKop:
    >>
    >> I like to do the following:
    >>
    >> namespace Windows { #include <windows.h> }
    >>
    >> that's keeps all of their crap away from your stuff, unfortunately
    >> though, the macros still get through.

    >
    > Unfortunately you'll have to add to your work-around definitions every
    > time you use something new from the API (relative to earlier work).
    >
    > Yes it can be done.
    >
    > No, in my experience it's not really worth it; it's a lot of work.
    >


    namespace Windows { #include <windows.h> }

    int main()
    {
    Windows::MessageBox("Hello World", ..... );
    }


    Seems simple enough to me!
     
    JKop, Jul 3, 2004
    #17
  18. Re: Off Topic: Request for Code Review

    * JKop:
    > Alf P. Steinbach posted:
    >
    > > * JKop:
    > >>
    > >> I like to do the following:
    > >>
    > >> namespace Windows { #include <windows.h> }
    > >>
    > >> that's keeps all of their crap away from your stuff, unfortunately
    > >> though, the macros still get through.

    > >
    > > Unfortunately you'll have to add to your work-around definitions every
    > > time you use something new from the API (relative to earlier work).
    > >
    > > Yes it can be done.
    > >
    > > No, in my experience it's not really worth it; it's a lot of work.
    > >

    >
    > namespace Windows { #include <windows.h> }
    >
    > int main()
    > {
    > Windows::MessageBox("Hello World", ..... );
    > }
    >
    >
    > Seems simple enough to me!


    It isn't.

    Wrapping library headers in a namespace nearly always means you have to
    add your own work-around definitions of various things, and all that work
    is for nothing when the next version of the library is released.

    For the header in question there are zillions such things.

    A naive first attemp can look like the code below (btw. now that I see that
    code again I'm a bit ashamed of having written such thing: it does not
    properly check pre-conditions and it fails to #define the relevant macro
    to obtain standard library compatibility from that header).

    Then you'll find more and more and more that requires "intervention" (C++
    legality aside)...



    // Tabs = indents = 4

    //////////////////////////////////////////////////////////////////////////////
    //
    // Module CPP_WINDOWS
    // Interface
    //
    //
    // Wraps the [windows.h] header file, providing C++ compatible declarations.
    // Also, places [windows.h] declarations in namespace "Api".
    //
    // Copyright (c) 1998 Alf P. Steinbach
    //

    #ifdef CPP_WINDOWS_H
    #error [cpp_windows.h] included twice.
    #else
    #define CPP_WINDOWS_H
    #endif






    //------------------------------------------ Dependencies:


    #ifndef FRAMEWORK_MACROS_H
    #include <misc/framework_macros.h>
    #endif

    #ifdef _INC_WINDOWS
    #pragma message( COMPILE_MESSAGE_WARNING "[windows.h] is already included." )
    #else


    // Ensure C++ compatible declarations.

    #define STRICT


    // Include all standard C library headers used by <windows.h> in the
    // global namespace. Including them in the "std" namespace is incompatible
    // with <windows.h> anyway.

    #ifndef _INC_STDARG
    #include <stdarg.h>
    #endif

    #ifndef _INC_STRING
    #include <string.h>
    #endif

    #ifndef _INC_CTYPE
    #include <ctype.h>
    #endif

    #ifndef _INC_STDLIB
    #include <stdlib.h>
    #endif


    // Place <windows.h> declarations in namespace "Api".

    BEGIN_API_NAMESPACE
    #include <windows.h>
    END_API_NAMESPACE


    // Redefine macros that refer unqualified to types defined in namespace.
    // E.g., IDI_WINLOGO is defined in terms of MAKEINTRESOURCE.

    #undef MAKEINTRESOURCEA
    #undef MAKEINTRESOURCEW
    #undef MAKEINTRESOURCE

    #define MAKEINTRESOURCEA(i) (Api::LPSTR)((Api::DWORD)((Api::WORD)(i)))
    #define MAKEINTRESOURCEW(i) (Api::LPWSTR)((Api::DWORD)((Api::WORD)(i)))
    #ifdef UNICODE
    #define MAKEINTRESOURCE MAKEINTRESOURCEW
    #else
    #define MAKEINTRESOURCE MAKEINTRESOURCEA
    #endif // !UNICODE

    #endif







    //------------------------------------------ Code:

    // None.

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Jul 3, 2004
    #18
  19. Ben Hanson

    JKop Guest

    Re: Off Topic: Request for Code Review

    Once again, macros are the devil.


    -JKop
     
    JKop, Jul 3, 2004
    #19
  20. Re: Off Topic: Request for Code Review

    E. Robert Tisdale wrote:

    >> Perhaps that is not what you intended,
    >> but that is the impression that comes across.

    >
    > Subscribers to the comp.lang.c++ newsgroup have wider interests
    > than Microsoft Windows C++.
    > Perhaps you don't mean to be inconsiderate or disrespectful of them
    > but that's the impression that comes across.


    Well. The good news is that no one here intends to be inconsiderate or
    disrespectful!

    I'll persist with this a little longer with this, because I think there is a
    serious point here.

    I generally apply common sense when deciding whether a posting would / would
    not be off topic. However, I do accept that if there are rules they should
    be followed. So I've had a look to see what rules there are. I couldn't
    find a charter for comp.lang.c++, but the FAQ says:-

    QUOTE
    Only post to comp.lang.c++ if your question is about the C++ language
    itself. For example, C++ code design, syntax, style, rules, bugs, etc.
    Ultimately this means your question must be answerable by looking into the
    C++ language definition as determined by the ISO/ANSI C++ Standard
    document, and by planned extensions and adjustments. Operating-specific
    questions (e.g., about Windows NT / 95 / 3.x, UNIX, etc.) should go to an
    operating-system-specific newsgroup (see below), not to comp.lang.c++.
    UNQUOTE

    Ben's original posting said "I'm trying to use best practice in my C++
    programming and would appreciateany advice anyone can give."  

    It seems to me that this question is within the guideline given by the FAQ.
    He asked what we thought about his C+ code, not whether we liked the
    Windows API or the way he was using it. Had he asked about the Windows API
    then I agree that the question would be off topic, both according to the
    FAQ guideline, and according to 'common sense'.

    It would be very difficult for him to write a useful program without using
    some kind of library / API. In his case he has chosen the Windows API.

    As Alf P Steinbach has pointed out, if we accept the argument that code that
    uses the Windows API cannot be discussed in this newsgroup, then by the
    same argument we would have to refuse to discuss any real code.

    It seems to me that Ben's code can be reviewed from a pure C++ perspective.
    For example, I note that he has used a static_cast. I generally try to
    avoid these if possible, and I might have suggested he try to get rid of
    it. Perhaps there is a very good reason for the static_cast, in which case
    my comment would have been invalid. The point is that discussion about the
    pros and cons of static_cast in particular cases would have been very much
    on topic for this newsgroup.

    Interestingly, in addition to ruling out discussion of operating system
    specific topics, a strict reading of the FAQ guideline would also seem to
    rule out discussion of any library, including the STL, which is not
    actually part of the C++ language itself.

    My common sense tells me that discussions of the STL are valid in this
    newsgroup.

    To summarise my main points:-

    * It seems to me that Ben's posting was on topic according to the FAQ
    guideline

    * Regardless of whether the posting was or was not actually off topic, my
    common sense tells me that it was reasonable for this newsgroup. It was
    about C++

    * I think that the newsgroup should try to be inclusive.

    --
    Chris Gordon-Smith
    London
    Homepage: http://graffiti.virgin.net/c.gordon-smith/
    Email Address: Please see my Home Page
     
    Chris Gordon-Smith, Jul 4, 2004
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Code Review Request

    , Jun 30, 2005, in forum: Java
    Replies:
    2
    Views:
    521
    Andrew Thompson
    Jun 30, 2005
  2. P Kenter

    Request for code review

    P Kenter, May 28, 2004, in forum: C++
    Replies:
    4
    Views:
    362
    P Kenter
    Jun 2, 2004
  3. Debashish Chakravarty

    request for code review

    Debashish Chakravarty, Nov 18, 2003, in forum: C Programming
    Replies:
    3
    Views:
    392
    Robert Stankowic
    Nov 18, 2003
  4. Adam Monsen

    code review request: basic file I/O

    Adam Monsen, Aug 16, 2004, in forum: C Programming
    Replies:
    9
    Views:
    341
    Arthur J. O'Dwyer
    Aug 19, 2004
  5. www
    Replies:
    51
    Views:
    1,486
Loading...

Share This Page