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

What's wrong with the following code snippet.

P: n/a
Hi,

Please tell me what's wrong with the following code.

private bool CheckIt( int iArg, bool fArg)

{

bool fIsGood = false;

bool fIsRight;

for (int i = 20; i < 30; i++ )

{

if (i == iArg)

fIsGood = true;

}

if (fArg == true)

fIsRight = true;

else

fIsRight = false;

if (fIsGood == true && fIsRight == true)

return true;

else

return false;

}
Sep 17 '06 #1
Share this Question
Share on Google+
7 Replies


P: n/a
"Jason Kid" <Ja******@microsoft.comwrote
Please tell me what's wrong with the following code.
Can you explain what it is you're trying to do?

I looked through your code, but I was unable to puzzle out what exactly you
were trying to accomplish.

--
Chris Mullins, MCSD.NET, MCPD:Enterprise
http://www.coversant.net/blogs/cmullins
Sep 17 '06 #2

P: n/a
"Jason Kid" <Ja******@microsoft.comwrote in message
news:Os**************@TK2MSFTNGP04.phx.gbl...
Hi,

Please tell me what's wrong with the following code.

private bool CheckIt( int iArg, bool fArg)

{

bool fIsGood = false;

bool fIsRight;

for (int i = 20; i < 30; i++ )

{

if (i == iArg)

fIsGood = true;

}
This can be written as

fIsGood = (iArg >= 20 && iArg < 30);
>
if (fArg == true)

fIsRight = true;

else

fIsRight = false;
This can be written as

fIsRight = fArg;

but it's not needed at all.
>
if (fIsGood == true && fIsRight == true)

return true;

else

return false;

}
This can be written as

return (fIsGood && fArg);

With further optimising your entire routine can be written in one line:

private bool CheckIt(int iArg, bool fArg)
{
return (fArg && iArg >= 20 && iArg < 30);
}

Michael
Sep 17 '06 #3

P: n/a
Another classic example of stupid abstract certification test questions. The
multiple choice answers never include "E. None of the above, no one would be
so stupid as to write this code and if they did and your employer approves
it then it's time to find a new employer."

Jon

"Chris Mullins" <cm******@yahoo.comwrote in message
news:el**************@TK2MSFTNGP04.phx.gbl...
"Jason Kid" <Ja******@microsoft.comwrote
>Please tell me what's wrong with the following code.

Can you explain what it is you're trying to do?

I looked through your code, but I was unable to puzzle out what exactly
you were trying to accomplish.

--
Chris Mullins, MCSD.NET, MCPD:Enterprise
http://www.coversant.net/blogs/cmullins


Sep 17 '06 #4

P: n/a
Not sure, but one thing that comes to mind is that it is in serious need of
refactoring and documentation.
private const int XYZRANGE_LOW = 20;
private const int XYZRANGE_HIGH = 30;
/// <summary>
/// Validates that the provided arguments are within XYZ range.
/// If <see cref="reqFlag" /is <code>false</code>, the returned value will
always be false.
/// </summary>
/// <param name="argInRange">Value to verify as being within XYZ
range.</param>
/// <param name="reqFlag">Flag to determine whether validation returns
/// <code>true</codeif validation passes.</param>
/// <returns>Boolean result of validation.</returns>
private bool ValidateForXyz(int argInRange, bool reqFlag)
{
return reqFlag && (argInRange >= XYZRANGE_LOW
&& argInRange < XYZRANGE_HIGH);
}

Jon
"Jason Kid" <Ja******@microsoft.comwrote in message
news:Os**************@TK2MSFTNGP04.phx.gbl...
Hi,

Please tell me what's wrong with the following code.

private bool CheckIt( int iArg, bool fArg)

{

bool fIsGood = false;

bool fIsRight;

for (int i = 20; i < 30; i++ )

{

if (i == iArg)

fIsGood = true;

}

if (fArg == true)

fIsRight = true;

else

fIsRight = false;

if (fIsGood == true && fIsRight == true)

return true;

else

return false;

}


Sep 17 '06 #5

P: n/a
Thanks Mike,

It seems like the code snippet has a lot of space to be further optimized
intead of logic error or serious potential performance impact.

Jason

"Michael C" <no****@nospam.comwrote in message
news:eG**************@TK2MSFTNGP03.phx.gbl...
"Jason Kid" <Ja******@microsoft.comwrote in message
news:Os**************@TK2MSFTNGP04.phx.gbl...
>Hi,

Please tell me what's wrong with the following code.

private bool CheckIt( int iArg, bool fArg)

{

bool fIsGood = false;

bool fIsRight;

for (int i = 20; i < 30; i++ )

{

if (i == iArg)

fIsGood = true;

}

This can be written as

fIsGood = (iArg >= 20 && iArg < 30);
>>
if (fArg == true)

fIsRight = true;

else

fIsRight = false;

This can be written as

fIsRight = fArg;

but it's not needed at all.
>>
if (fIsGood == true && fIsRight == true)

return true;

else

return false;

}

This can be written as

return (fIsGood && fArg);

With further optimising your entire routine can be written in one line:

private bool CheckIt(int iArg, bool fArg)
{
return (fArg && iArg >= 20 && iArg < 30);
}

Michael

Sep 17 '06 #6

P: n/a
Michael C wrote:

if (fArg == true)

fIsRight = true;

else

fIsRight = false;

This can be written as

fIsRight = fArg;
This should generate a compiler error as fArg is an int and flsRight is
bool.

Sep 18 '06 #7

P: n/a
Michael C wrote:
if (fIsGood == true && fIsRight == true)

return true;

else

return false;

}
Oh, and if (fIsGood == true && fIsRight == true) is redundant.

if (fIsGood && fIsRight)
But, of course what you wrote was better:

return (fIsGood && fIsRight);

Sep 18 '06 #8

This discussion thread is closed

Replies have been disabled for this discussion.