What's wrong with the following code snippet.

[Jason Kid]

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;

}

 

[Chris Mullins]

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.

 

[Michael C]

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

 

[Jon Davis]

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

 

[Jon Davis]

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</code> if 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]

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

 

[Chris Dunaway]

This can be written as

fIsRight = fArg;

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

 

[Chris Dunaway]

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

if (fIsGood && fIsRight)


But, of course what you wrote was better:

return (fIsGood && fIsRight);