what is a better style?

Discussion in 'C++' started by Nobody, Oct 30, 2005.

  1. Nobody

    Nobody Guest

    Lets say I have a class that is only available if a specific DLL (a.dll) is
    present. I can't link to that DLL through lib files or my app will fail on
    any machine that doesn't have a.dll.

    So I do LoadLibrary()'s and keep function pointers in my wrapper class...
    Which is a better style?

    DWORD CClass::SomeFunc()
    {
    if (m_pfn != NULL)
    return (*m_pfn)(parm1, parm2);

    return (DWORD) -1;
    }

    or

    DWORD CClass::SomeFunc()
    {
    ASSERT(m_pfn != NULL);
    return (*m_pfn)(parm1, parm2);
    }

    I guess the first version fails "more graciously" if the developer calls it
    incorrectly, but the 2nd version isn't returning "made up" return codes and
    is more "in your face" during development.
     
    Nobody, Oct 30, 2005
    #1
    1. Advertising

  2. Nobody wrote:
    > Lets say I have a class that is only available if a specific DLL (a.dll) is
    > present. I can't link to that DLL through lib files or my app will fail on
    > any machine that doesn't have a.dll.
    >
    > So I do LoadLibrary()'s and keep function pointers in my wrapper class...
    > Which is a better style?
    >
    > DWORD CClass::SomeFunc()
    > {
    > if (m_pfn != NULL)
    > return (*m_pfn)(parm1, parm2);
    >
    > return (DWORD) -1;
    > }
    >
    > or
    >
    > DWORD CClass::SomeFunc()
    > {
    > ASSERT(m_pfn != NULL);
    > return (*m_pfn)(parm1, parm2);
    > }
    >
    > I guess the first version fails "more graciously" if the developer calls it
    > incorrectly, but the 2nd version isn't returning "made up" return codes and
    > is more "in your face" during development.


    Assertions are used during development to catch "that's not supposed to
    happen" errors, such as bad arguments to a function. They disappear
    when the program is compiled in "release" mode, so they are of no use
    to convey information to users.

    Returning error codes (or sometimes throwing exceptions) are common
    ways to signal a "valid" error in a program. It is then possible to
    display a user-friendly message, recover or fail.

    As you can see, both ways are not necessarily exclusives. Actually, it
    is common practice to use both assertions and error codes in the same
    function. If it is normal (or predictable) for an error to happen, you
    should not use (only) assertions to catch them, since you don't want a
    user to see them.

    Remember to be both user-friendly (try to recover or display an
    informative message and fail graciously) and developper-friendly
    (convey as much informations you can, such as with compile-time or
    run-time assertions).


    Jonathan
     
    Jonathan Mcdougall, Oct 30, 2005
    #2
    1. Advertising

  3. "Nobody" <> wrote in message
    news:5dV8f.779$5N1.161@dukeread08...
    : Lets say I have a class that is only available if a specific DLL
    (a.dll) is
    : present. I can't link to that DLL through lib files or my app will
    fail on
    : any machine that doesn't have a.dll.
    :
    : So I do LoadLibrary()'s and keep function pointers in my wrapper
    class...
    : Which is a better style?
    :
    : DWORD CClass::SomeFunc()
    : {
    : if (m_pfn != NULL)
    : return (*m_pfn)(parm1, parm2);
    :
    : return (DWORD) -1;
    : }
    This function is ok if a value such as (DWORD)-1 makes sense
    and can be interpreted as an error condition by the caller.
    But as you mentioned, such an "invalid" result value may not
    always exist -- so a separate way to report errors may be needed.

    And this is exactly what exceptions are for.
    I would actually write this function as:
    DWORD CClass::SomeFunc()
    {
    if( m_pfn == NULL )
    throwLibraryNotLoadedError(); // calls throw(..smthg..)
    return (*m_pfn)(parm1, parm2);
    }

    : or
    :
    : DWORD CClass::SomeFunc()
    : {
    : ASSERT(m_pfn != NULL);
    : return (*m_pfn)(parm1, parm2);
    : }

    So what happens in release mode, assuming that ASSERT checks
    are then disabled, if that function is called when m_pfn==0 ?
    You will be getting undefined behavior (hopefully just a
    crash of your program).
    Can this ever be a good idea ?

    : I guess the first version fails "more graciously" if the developer
    calls it
    : incorrectly, but the 2nd version isn't returning "made up" return
    codes and
    : is more "in your face" during development.

    My approach is to throw an exception if one of the preconditions
    of the function is violated (for instance, if no DLL was loaded).
    To help with debugging, I may also want to trigger an assertion
    failure, breakpoint or warning at debug time (i.e. within the function
    throwLibraryNotLoadedError() ).

    Ivan
    --
    http://ivan.vecerina.com/contact/?subject=NG_POST <- email contact
    form
    Brainbench MVP for C++ <> http://www.brainbench.com
     
    Ivan Vecerina, Oct 30, 2005
    #3
  4. Nobody

    Greg Guest

    Nobody wrote:
    > Lets say I have a class that is only available if a specific DLL (a.dll) is
    > present. I can't link to that DLL through lib files or my app will fail on
    > any machine that doesn't have a.dll.
    >
    > So I do LoadLibrary()'s and keep function pointers in my wrapper class...
    > Which is a better style?
    >
    > DWORD CClass::SomeFunc()
    > {
    > if (m_pfn != NULL)
    > return (*m_pfn)(parm1, parm2);
    >
    > return (DWORD) -1;
    > }
    >
    > or
    >
    > DWORD CClass::SomeFunc()
    > {
    > ASSERT(m_pfn != NULL);
    > return (*m_pfn)(parm1, parm2);
    > }
    >
    > I guess the first version fails "more graciously" if the developer calls it
    > incorrectly, but the 2nd version isn't returning "made up" return codes and
    > is more "in your face" during development.


    The difference between these two versions is more than an issue of
    style. Each is making a very different statement about the program.
    Either one may be correct, but not both.

    The first version states that m_pfn could be NULL while this program is
    executing; so it is necessary for the program to check for that
    possibility and report it to the caller.

    The second version states that the program is certain that m_pfn will
    never be NULL while it is executing. The program's behavior is
    therefore defined only when m_pfn is not NULL. Should that assumption
    ever turn out not to be true, then the program's behavior will be
    undefined. Ideally the program would crash quickly without causing any
    harm.

    Greg
     
    Greg, Oct 30, 2005
    #4
  5. Nobody

    Mirek Fidler Guest

    Nobody wrote:
    > Lets say I have a class that is only available if a specific DLL (a.dll) is
    > present. I can't link to that DLL through lib files or my app will fail on
    > any machine that doesn't have a.dll.
    >
    > So I do LoadLibrary()'s and keep function pointers in my wrapper class...
    > Which is a better style?
    >
    > DWORD CClass::SomeFunc()
    > {
    > if (m_pfn != NULL)
    > return (*m_pfn)(parm1, parm2);
    >
    > return (DWORD) -1;
    > }
    >
    > or
    >
    > DWORD CClass::SomeFunc()
    > {
    > ASSERT(m_pfn != NULL);
    > return (*m_pfn)(parm1, parm2);
    > }
    >
    > I guess the first version fails "more graciously" if the developer calls it
    > incorrectly, but the 2nd version isn't returning "made up" return codes and
    > is more "in your face" during development.


    I guess later is better - because not every function returns DWORD that
    can be interpreted. Your code has to verify the presence of .dll abefore
    calling methods anyway, so you are just duplicating this there.

    BTW, in our system we have introduced nice related tool to automatize
    generating such wrapper objects. To create such object, we provide
    ".dli" file with content like (actual example is for Lotus Notes client
    ..dll)

    FN(WORD, OSLoadString, (HMODULE hModule, STATUS StringCode, char
    *retBuffer, WORD BufferLength))
    FN(WORD, OSTranslate, (WORD TranslateMode, char far *In, WORD
    InLength, char far *Out, WORD OutLength))
    FN(STATUS, NotesInitExtended, (int argc, char **argv))
    FN(STATUS, OSPathNetConstruct, (char *PortName, char *ServerName, char
    far *FileName, char *retPathName))
    FN(STATUS, NSFDbOpen, (char far *PathName, DBHANDLE far *rethDB))
    FN(STATUS, NSFDbClose, (DBHANDLE hDB))
    ............

    - basically, this is somewhat "reparsed" header file for .dll. Then place

    #define DLLFILENAME "nnotes.dll"
    #define DLIMODULE NOTES
    #define DLIHEADER <notes/notes.dli>
    #define DLLCALL LNPUBLIC
    #include <Core/dli_header.h>

    to common header file and

    #define DLLFILENAME "nnotes.dll"
    #define DLIMODULE NOTES
    #define DLIHEADER <notes/notes.dli>
    #define DLLCALL LNPUBLIC
    #include <Core/dli_header.h>

    to some .cpp file.

    This creates global function NOTES() returning object instance that has
    all .dll functions described in .dli file defined as its methods.
    Moreover, it has operator bool that can be used to test whether .dll is
    present:

    char h[256];
    if(NOTES())
    NOTES().OSLoadString(GetModuleHandle(NULL), ERR(nError), h, 255);


    (http://upp.sf.net)

    Mirek
     
    Mirek Fidler, Oct 30, 2005
    #5
    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. Steven T. Hatton

    Would a static_cast be better style here?

    Steven T. Hatton, Apr 16, 2004, in forum: C++
    Replies:
    26
    Views:
    743
    Jerry Coffin
    Apr 19, 2004
  2. Peter Bencsik
    Replies:
    2
    Views:
    855
  3. Ken Varn
    Replies:
    0
    Views:
    489
    Ken Varn
    Apr 26, 2004
  4. Andrew Thompson
    Replies:
    8
    Views:
    158
    Premshree Pillai
    Jun 7, 2005
  5. Replies:
    2
    Views:
    60
    Mark H Harris
    May 13, 2014
Loading...

Share This Page