One Thousand Dollars an Hour

I had a quick look at his SSToolkit framework which he published on github. I reviewed he source code because he said he is doing iOS code reviews for $1000/hour. After five minutes or so I stopped taking him serious. In my opinion his code is good but really far from brilliant. There are even some serious flaws in his code - at least in my opinion. Here is a short and imcomplete summary of his code ( https://github.com/samsoffes/sstoolkit ):

The method -randomObject (NSArray+SSToolkitAdditions) is supposed to return a random object but when he is generating a random number he is generating non-uniform random numbers.

In general his category methods are not prefixed. This will definitely break something sooner or later.

He is not using the designated initializer or NSDate - although it would make sense in -dateValue (NSNumber-SSToolkitAdditions)

He is drawing patterns in -drawRect: where he could simply set the background image to a pattern color.

He is not using the shortcut functions for handling CGRects.

He has a file containing utility drawing functions (SSDrawingUtilities). This file contains functions like CGRectSetWidth, CGRectSetOrigin and so on which pollute the CG namespace which is owned by Apple...

>The method -randomObject (NSArray+SSToolkitAdditions) is supposed to return a random object but when he is generating a random number he is generating non-uniform random numbers.

You mean because of the modulo operator, presumably, and yes, the technically correct way to do it is to retry if the raw random number is larger than N*floor(UINT32_MAX/N). Still, that's a pretty minor bug considering what a minor deviation from uniformity it causes.

>In general his category methods are not prefixed. This will definitely break something sooner or later.

Yeah, this is a common mistake for people making "additions" frameworks, and it just strikes me as egotistical. When I was doing ObjC development, I'd even prefix my own private category methods, because hey, you never know.

Given the response this is generating, I have to disagree.

Edit: I just realized for my math below I used 2^32 as UINT32_MAX instead of 2^32 - 1. The rest of my reasoning holds up though.

>Especially with small arrays his implementation will return the the 0th or 1st element much much more often than other elements.

Much, much more often? Correct me if my reasoning is wrong, but for a, say, 6 element array, this bug would make a difference less than 0.0000001% of the time ((UINT32_MAX%6)/UINT32_MAX). For a 3 element array, it would make a difference about 0.000000023% of the time. Maybe you mean by small "less than 100 elements", in which case the worst case is 96 elements, where it happens a whopping 0.0000015% of the time.

Indeed, the problem is only pronounced for large arrays, because that's when there's an opportunity for UINT32_MAX%N to be large (since A%B no case where the 0th and 1st element are significantly skewed ahead of everything else. The bias is characterized by the last few elements of the array being selected significantly less often, but the "significantly" part only kicks in for very big arrays.

In fact, the first array length for which you will even see a difference 0.01% of the time would be 430,142 elements [ed: this holds up even adjusting UINT32_MAX to the correct value]. Considering that this is a framework intended to be used for iOS development, I think once your NSArray has grown that large, you might have more important fish to fry than worrying about a small bias in your randomization.

that's not necessary at all...