Stuart Breckenridge

Code Review

When I first got interested in programming – around 2009 – the first app I made was a little utility to help you pick your lottery numbers. It was called Lucky Dip and it sold exclusively in the UK App Store and it very quickly went from being £0.69 to being free.1

In the spirit of continual improvement I’m reviewing my younger self’s code, and boy is it a painful experience.

The problem statement: generate a set of six unique numbers between 1 and 49.


Younger Self

I’m sure I attacked the problem in the following way:

  • find out how to generate numbers within a specific range;
  • generate six of them; then,
  • compare them all against each other individually to make sure they are unique.

This is what my code looked like:

- (NSArray *)generateLottoNumbers
{
        // Generate Numbers
    int n1 = arc4random() % 49 + 1;
    int n2 = arc4random() % 49 + 1;
    int n3 = arc4random() % 49 + 1;
    int n4 = arc4random() % 49 + 1;
    int n5 = arc4random() % 49 + 1;
    int n6 = arc4random() % 49 + 1;
    
        // Check uniqueness
    if (n1 == n2 || n1 == n3 || n1 == n4 || n1 == n5 || n1 == n6 ||
        n2 == n3 || n2 == n4 || n2 == n5 || n2 == n6 ||
        n3 == n4 || n3 == n5 || n3 == n6 ||
        n4 == n5 || n4 == n6 ||
        n5 == n6)
        {
            return [self generateLottoNumbers]; // Duplicates detected.
        }
    
    	// Return a sorted array.
    else
        {
        NSMutableArray *numbers = [NSMutableArray arrayWithObjects:@(n1), @(n2), @(n3), @(n4), @(n5), @(n6),  nil];
        NSArray *array = [numbers sortedArrayUsingSelector:@selector(compare:)]; 
        return array;
        }
}

The code was then called with:

	NSArray *numbers = [self generateLottoNumbers];

This existed in each view controller in a different incarnation, suitable to each lottery game (typed out every time).2 Talk about not having reusable code and massive view controllers.

Current Self

What I’d do now is create reusable code so each view controller for each lottery game called the same function. I’d start with a protocol:

protocol LottoNumbers
{
    func generateLottoNumbers(count:Int,maxRange:UInt32) -> [Int]
}

I’d then extend UIViewController to conform to the LottoNumbers protocol3. The function makes use of a Set to ensure that there are the correct amount of unique numbers:

extension UIViewController:LottoNumbers
{
    func generateLottoNumbers(count:Int,maxRange:UInt32) -> [Int]
    {
        var i = 0
        let uniqueSet = NSMutableSet(capacity: count)
        var numberArray = [Int]()
        
        repeat {
            let genNumber = (Int(arc4random() % maxRange + 1))
            uniqueSet.addObject(genNumber)
            numberArray.append(genNumber)
            ++i
        } while i < count
        
        if uniqueSet.count != count
        {
            return self.generateLottoNumbers(count, maxRange: maxRange)
        }
        
        numberArray.sortInPlace({$0<$1})
        
        return numberArray
    }
}

This code can be reused depending on the rules of the game and it also prevents having a massive view controller:

let numbers = generateLottoNumbers(6, maxRange: 49) // Lotto.
let numbers = generateLottoNumbers(5, maxRange: 50) // Euromillions.
// etc...

There are always better ways of doing things. However, the original version worked and that’s all that mattered at the time.

  1. I had lofty ambitions of World domination that didn’t last. ↩︎

  2. There were three others: Hotpicks, Thunderball, and Euromillions. ↩︎

  3. This could also be achieved with an Objective-C tags. ↩︎


Supported by