By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
455,433 Members | 1,842 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 455,433 IT Pros & Developers. It's quick & easy.

Redundant behavior in coding guideline

P: n/a
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 }

Nov 15 '05 #1
Share this Question
Share on Google+
2 Replies


P: n/a
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.
Nov 15 '05 #2

P: n/a
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
Nov 15 '05 #3

This discussion thread is closed

Replies have been disabled for this discussion.