Recently I was posed the following question:

Write a piece of code that prints all odd integer numbers between 1 and 99

This really isn't a difficult question but it still requires some thought. When I'm posed with any question I like to break things down into their constituent parts.

Here's the process I went through:

Okay, so I'll define two variables for a start and end value and there's going to have to be a loop.
[csharp]int startValue = 1;
int endValue = 99;
for(int i = startValue;
i <= endValue;
i++)
{
// work out if "i" is an odd number
}[/csharp]
Now, for the odd number detection. And... after a few umms and errrs ... I'm going to have to mod 2 (%2) the current value of i to work out if the value is odd. More ... umms and errs. Okay, I've finally worked out that if something mod 2 is not equal to 0 it's clearly an odd number. This took me longer than it should have but never mind. Once I've detected if i is an odd number I'll then put the odd number into a list for use later.
[csharp]
int startValue = 1;
int endValue = 99;
IList<int> oddValues = new List&lt;int&gt;();
for(int i = startValue;
i &lt;= endValue;
i++)
{
if(i%2 != 0)
{
oddValues.Add(i);
}
}
[/csharp]
Those of you that are good at these little puzzles, or just think this is way too easy, might already be screaming at me about one of the following:

• Why are you using a IList</code>, why don't you just print the value?</li>
• Odd numbers are always 2 apart so why aren't you just increment i by 2 using i+=2?
</ul>


I admit it, I missed the second point and that is a bit silly of me. However, what I have starting doing is separating the concerns of the piece of code. The code does two things; it detects the odd number and it prints the odd numbers. Those are two very distinct things. So, my code now looks like this:
[csharp]
int startValue = 1;
int endValue = 99;
IList<int> oddNumbers = new List&lt;int&gt;();
for(int i = startValue;
i &lt;= endValue;
i++)
{
if(i%2 != 0)
{
oddNumbers.Add(i);
}
}

string oddNumbersList = string.Join(",", oddNumbers.ToArray());
Console.WriteLine(oddNumbersList );
[/csharp]
I'm now going to refactor this further so I'll put the two different pieces of functionality into different methods. I'll rename the i, startValue and endValue variables to be something a bit more useful; say numberToCheck, startNumber and endNumber. I'll also create another helper for the odd number checking named IsOddNumber:
[csharp]
IList<int> GetOddNumbersBetween(int startNumber, int endNumber)
{
IList oddValues = new List&lt;int&gt;();
for(int numberToCheck = startNumber;
numberToCheck &lt;= endNumber;
numberToCheck++)
{
if(IsOddNumber(numberToCheck) == true)
{
oddValues.Add(numberToChecki);
}
}
return oddValues;
}

bool IsOddNumber(int number)
{
return (number % 2 == 1);
}

void PrintOddNumbersBetween(int startNumber, int endNumber)
{
IList<int> oddNumbers = GetOddNumbersBetween(startNumber, endNumber);
string oddNumbersList = string.Join(",", oddNumbers.ToArray());
Console.WriteLine(oddNumbersList);
}
[/csharp]
Let's say I then notice the second point you've been screaming at me about (Odd numbers are always 2 apart so why aren't you just increment i by 2 using i+=2) that I mentioned above? In practice I should notice this sort of thing either when I give the code a complete review, or one of my peers spots it. When I see this problem I decide to update the for loop, as noted, and I then see that I possibly don't need the if(IsOddNumber(i) == true) statement. Although it would pain me to do this, since it's a lovely little method, I would need to consider deleting it. But then it strikes me, I'm no longer just solving the "odd numbers between 1 and 99 problem" so I can't just assume that the startNumber is going to be an odd number. I need to make sure that it's an odd number so I'll create another small utility method for that called EnsureOddNumber which will check if the value passed is an odd number, and if not return the next odd number (I'd like to rethink the name of this method).

[csharp]
IList<int> GetOddNumbersBetween(int startNumber, int endNumber)
{
startNumber = EnsureOddNumber(startNumber);

IList<int> oddValues = new List&lt;int&gt;();
for(int numberToCheck = startNumber;
numberToCheck &lt;= endNumber;
numberToCheck+=2)
{
oddValues.Add(numberToCheck);
}
return oddValues;
}

int EnsureOddNumber(int number)
{
if( IsOddNumber(startNumber) == false )
{
startNumber++;
}
return startNumber;
}

bool IsOddNumber(int number)
{
return (number % 2 == 1);
}

void PrintOddNumbersBetween(int startNumber, int endNumber)
{
IList<int> oddNumbers = GetOddNumbersBetween(startNumber, endNumber);
string oddNumbersList = string.Join(",", oddNumbers.ToArray());
Console.WriteLine(oddNumbersList);
}
[/csharp]

Now, when I look at this code I get a warm feeling because I feel that it solves the problem, it's well engineered, the concerns are separated and the code is completely self documenting.

There are a few comments that people may have here:

The question asked specifically to print odd values between 1 and 99 and you've done more than was required.

Although my answer does satisfy the original question have I over engineered things? The question does specifically ask us to print odd values between 1 and 99 so maybe I should have created a function that just satisfied that requirement.

[csharp]
void PrintOddNumbersBetween1And99()
{
for(int i = 1;
i &lt;= 99;
i+=2)
{
Console.WriteLine(i + " ");
}
}
[/csharp]

And I'd have to admit that this very short piece of code exactly answers the question. But I'd also argue that there is very little chance of this code being reused. Don't get me wrong, you definitely shouldn't over engineer things but there should be some scope for code reuse.

For such a simple problem you've over engineered this.

or

There's a better solutions that that... it's not efficient

Is creating four methods over engieering? Does my code require any comments to provide documentation? There may well be a more performant solution to this, but that's not my point. My point is the way of approaching a question: the thought processes involved in understanding the problem and breaking it down to separate concerns, making it easy to read, reusable and self documenting. If the code that worked out the odd numbers is not efficient it could easily be changed in one place without impacting the interface, the other methods within the class, or the overall functionality.

Some people may jump to the simplest solution but I think the way i've described approaching and solving the problem demonstrates good practice. If I'm completely honest I would normally approach the development of something such as this by writing a test case first since I practice TDD but that can wait for another blog post.