Redundant behavior in coding guideline

Discussion in 'C Programming' started by lovecreatesbeauty, Oct 27, 2005.

  1. Some of the coding guideline are mandatory, and even the format or
    layout of the text of the source code also should be followed. There's
    plenty of codes like the following snippet.

    Do you think the "re-evaluating the previous condition" is necessary
    and should be avoided though it's one of your coding guideline? And
    yes, there's plenty of code like this in many projects.



    78 /* Error handling */
    79 if ((NULL == p_SrcNm) || (NULL == p_DestNm))
    80 {
    81 lb_RetVal = false;
    82
    83 /* some other things special here ... */
    84
    85 if (NULL == p_SrcNm) /* Re-evaluating the previous
    condition */
    86 {
    87 /* some other things special here ... */
    88
    89 ErrorEntryAdd(p_SrcNm);
    90 ErrorHandle();
    91 }
    92
    93 if (NULL == p_DestNm) /* Re-evaluating the previous
    condition */
    94 {
    95 /* some other things special here ... */
    96
    97 ErrorEntryAdd(p_DestNm);
    98 ErrorHandle();
    99 }
    100 }
    lovecreatesbeauty, Oct 27, 2005
    #1
    1. Advertising

  2. lovecreatesbeauty

    Skarmander Guest

    lovecreatesbeauty wrote:
    > Some of the coding guideline are mandatory, and even the format or
    > layout of the text of the source code also should be followed. There's
    > plenty of codes like the following snippet.
    >
    > Do you think the "re-evaluating the previous condition" is necessary
    > and should be avoided though it's one of your coding guideline? And
    > yes, there's plenty of code like this in many projects.
    >
    >
    >
    > 78 /* Error handling */
    > 79 if ((NULL == p_SrcNm) || (NULL == p_DestNm))
    > 80 {
    > 81 lb_RetVal = false;
    > 82
    > 83 /* some other things special here ... */
    > 84
    > 85 if (NULL == p_SrcNm) /* Re-evaluating the previous
    > condition */


    No, it's not re-evaluating. (NULL == p_SrcNm) || (NULL == p_DestNm) does
    not equal or even imply (NULL == p_SrcNm).

    If you mean that the boolean expression (NULL == p_SrcNm) is
    "recalculated", then yes. However, comparing a pointer to NULL is such a
    trivial affair that it doesn't warrant a separate boolean to record the
    outcome rather than just repeating the check.

    > 86 {
    > 87 /* some other things special here ... */
    > 88
    > 89 ErrorEntryAdd(p_SrcNm);
    > 90 ErrorHandle();
    > 91 }
    > 92
    > 93 if (NULL == p_DestNm) /* Re-evaluating the previous
    > condition */
    > 94 {
    > 95 /* some other things special here ... */
    > 96
    > 97 ErrorEntryAdd(p_DestNm);
    > 98 ErrorHandle();
    > 99 }
    > 100 }
    >


    I'm pretty sure the calls to "ErrorEntryAdd" are wrong, because they're
    just adding NULL pointers as whatever an "error entry" is. That seems
    rather useless. Are you sure you didn't forget quotes? Don't you need to
    pass a string with an error description or something?

    S.
    Skarmander, Oct 27, 2005
    #2
    1. Advertising

  3. lovecreatesbeauty

    Netocrat Guest

    On Thu, 27 Oct 2005 12:58:45 +0200, Skarmander wrote:
    > lovecreatesbeauty wrote:
    >> Some of the coding guideline are mandatory, and even the format or
    >> layout of the text of the source code also should be followed. There's
    >> plenty of codes like the following snippet.
    >>
    >> Do you think the "re-evaluating the previous condition" is necessary
    >> and should be avoided though it's one of your coding guideline? And
    >> yes, there's plenty of code like this in many projects.
    >>
    >> 78 /* Error handling */
    >> 79 if ((NULL == p_SrcNm) || (NULL == p_DestNm))
    >> 80 {
    >> 81 lb_RetVal = false;
    >> 82
    >> 83 /* some other things special here ... */
    >> 84
    >> 85 if (NULL == p_SrcNm) /* Re-evaluating the previous
    >> condition */

    >
    > No, it's not re-evaluating. (NULL == p_SrcNm) || (NULL == p_DestNm) does
    > not equal or even imply (NULL == p_SrcNm).
    >
    > If you mean that the boolean expression (NULL == p_SrcNm) is
    > "recalculated", then yes. However, comparing a pointer to NULL is such a
    > trivial affair that it doesn't warrant a separate boolean to record the
    > outcome rather than just repeating the check.


    An alternative structuring would remove the outer conditional and repeat
    the code indicated by the outermost "some other things special here"
    (possibly including "lb_RetVal = false;"), within each inner conditional
    body.

    The drawbacks are dual-maintenance of that code and the likelihood of
    extra generated machine code.

    The dual-maintenance could be dealt with by encapsulating the code in a
    function; the extra complexity is a drawback to this approach, and unless
    the function were inlined, the function call overhead would likely exceed
    that of the removed conditional.

    Since this is error-handling code, performance is likely not critical and
    the current structuring seems preferable.

    >> 86 {
    >> 87 /* some other things special here ... */
    >> 88
    >> 89 ErrorEntryAdd(p_SrcNm);
    >> 90 ErrorHandle();
    >> 91 }
    >> 92
    >> 93 if (NULL == p_DestNm) /* Re-evaluating the previous
    >> condition */
    >> 94 {
    >> 95 /* some other things special here ... */
    >> 96
    >> 97 ErrorEntryAdd(p_DestNm);
    >> 98 ErrorHandle();
    >> 99 }
    >> 100 }

    [...]
    --
    http://members.dodo.com.au/~netocrat
    Netocrat, Oct 27, 2005
    #3
    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. John Salerno

    style guideline for naming variables?

    John Salerno, Mar 17, 2006, in forum: Python
    Replies:
    2
    Views:
    264
    Duncan Smith
    Mar 18, 2006
  2. Vyom

    macro style guideline

    Vyom, Nov 21, 2004, in forum: C Programming
    Replies:
    7
    Views:
    307
    Dan Pop
    Nov 23, 2004
  3. lovecreatesbeauty

    Redundant behavior in coding guideline

    lovecreatesbeauty, Oct 27, 2005, in forum: C Programming
    Replies:
    0
    Views:
    395
    lovecreatesbeauty
    Oct 27, 2005
  4. lovecreatesbeauty
    Replies:
    17
    Views:
    578
    Jordan Abel
    Jan 1, 2006
  5. sloan

    Re: Redundant coding...

    sloan, Dec 22, 2008, in forum: ASP .Net
    Replies:
    1
    Views:
    306
Loading...

Share This Page