coding style question

D

DBC User

I have the following code which does
1. Make sure the array of numbers are in decending order starting from
startNumber.
2. Make sure the total numbers in the array is equal to the
totalNumbers passed to the method.

Following code works, but for me it looks like there must be a better
way of doing this code. If any of you have any suggestions, please let
me know.

private bool ValidateNumbers(int[] numbers, int startNumber, int
totalNumbers)
{
bool returnFlag = true;
int sNum = startNumber;

foreach(int num in numbers)
{
if (num == sNum)
sNum--;
else
{
returnFlag = false;
break;
}
}

if (returnFlag && (numbers.Length != totalNumbers))
returnFlag = false;

return returnFlag;
}
 
C

Chris Shepherd

DBC said:
I have the following code which does
1. Make sure the array of numbers are in decending order starting from
startNumber.
2. Make sure the total numbers in the array is equal to the
totalNumbers passed to the method.

Following code works, but for me it looks like there must be a better
way of doing this code. If any of you have any suggestions, please let
me know.

Maybe something as simple as (assuming C# 2.0 or newer) pulling out just
the array of items from startNumber to the end first, dump them into a
List, and then sort the List?

List<int> numList = new List<int>(numbers);
numList.Sort();

Chris.
 
J

Jon Skeet [C# MVP]

I have the following code which does
1. Make sure the array of numbers are in decending order starting from
startNumber.
2. Make sure the total numbers in the array is equal to the
totalNumbers passed to the method.

Following code works, but for me it looks like there must be a better
way of doing this code. If any of you have any suggestions, please let
me know.

Unlike some people, I believe multiple return points are fine and
often improve readability.
In this case, you can "early out" very easily without having to keep
an extra variable around:

bool ValidateNumbers(int[] numbers, int startNumber, int totalNumbers)
{
if (numbers.Length != totalNumbers)
{
return false;
}

for (int i=0; i < totalNumbers; i++)
{
if (numbers != startNumber-i)
{
return false;
}
}
return true;
}

Jon
 
C

Chris Shepherd

DBC said:
I have the following code which does
1. Make sure the array of numbers are in decending order starting from
startNumber.
2. Make sure the total numbers in the array is equal to the
totalNumbers passed to the method.

Following code works, but for me it looks like there must be a better
way of doing this code. If any of you have any suggestions, please let
me know.

Expanding on my previous post:

List<int> numList = new List<int>(numbers);
int lenToSort = numList.Count - startNumber;
numList.Sort(startNumber, lenToSort, null);
numList.Reverse(startNumber, lenToSort);

That seems to do the trick described to me.

Chris.
 
D

DBC User

Expanding on my previous post:

List<int> numList = new List<int>(numbers);
int lenToSort = numList.Count - startNumber;
numList.Sort(startNumber, lenToSort, null);
numList.Reverse(startNumber, lenToSort);

That seems to do the trick described to me.

Chris.

Thanks, I do not want to sort, I would like to check if the list is in
sorted order.
 
C

Chris Shepherd

DBC said:
Thanks, I do not want to sort, I would like to check if the list is in
sorted order.

Ahhh. Different interpretations of "make sure" then I guess. Jon's reply
should suit you then.

Chris.
 
D

DBC User

I have the following code which does
1. Make sure the array of numbers are in decending order starting from
startNumber.
2. Make sure the total numbers in the array is equal to the
totalNumbers passed to the method.
Following code works, but for me it looks like there must be a better
way of doing this code. If any of you have any suggestions, please let
me know.

Unlike some people, I believe multiple return points are fine and
often improve readability.
In this case, you can "early out" very easily without having to keep
an extra variable around:

bool ValidateNumbers(int[] numbers, int startNumber, int totalNumbers)
{
if (numbers.Length != totalNumbers)
{
return false;
}

for (int i=0; i < totalNumbers; i++)
{
if (numbers != startNumber-i)
{
return false;
}
}
return true;

}

Jon


Thanks and I am one of the people would like have one entry and one
exit. But I agree the early exit is an excellent option.
 
D

DBC User

I have the following code which does
1. Make sure the array of numbers are in decending order starting from
startNumber.
2. Make sure the total numbers in the array is equal to the
totalNumbers passed to the method.
Following code works, but for me it looks like there must be a better
way of doing this code. If any of you have any suggestions, please let
me know.

Unlike some people, I believe multiple return points are fine and
often improve readability.
In this case, you can "early out" very easily without having to keep
an extra variable around:

bool ValidateNumbers(int[] numbers, int startNumber, int totalNumbers)
{
if (numbers.Length != totalNumbers)
{
return false;
}

for (int i=0; i < totalNumbers; i++)
{
if (numbers != startNumber-i)
{
return false;
}
}
return true;

}

Jon


Jon,

I rewrote the code based on your suggestion

if (numbers.Length == totalNumbers)
{
for (int i=0; i < totalNumbers; i++)
{
if (numbers != startNumber-i)
{
return false;
}
}
return true;
}
return false;

Will this be same as you thought?
 
J

Jon Skeet [C# MVP]

I rewrote the code based on your suggestion

if (numbers.Length == totalNumbers)
{
for (int i=0; i < totalNumbers; i++)
{
if (numbers != startNumber-i)
{
return false;
}
}
return true;
}
return false;

Will this be same as you thought?


Not really - why continue at all if you've got the wrong number of
elements?
At that point you know the complete result, so the code becomes
simpler to exit at that point.

Code written with this in mind tends to have fewer levels of
indentation, which I find helps readability.

Jon
 
D

DBC User

<snip>




I rewrote the code based on your suggestion
if (numbers.Length == totalNumbers)
{
for (int i=0; i < totalNumbers; i++)
{
if (numbers != startNumber-i)
{
return false;
}
}
return true;
}
return false;

Will this be same as you thought?

Not really - why continue at all if you've got the wrong number of
elements?
At that point you know the complete result, so the code becomes
simpler to exit at that point.

Code written with this in mind tends to have fewer levels of
indentation, which I find helps readability.

Jon- Hide quoted text -

- Show quoted text -


Excellent, thanks for the feedback.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top