POI Spremberg

Hikari had just left Apps R' Us when our submitter, Steve, was asked by the CEO to review some of his code. Now, Steve wasn't on the same project as Hikari, but he had a reputation for being thorough and concise, while Hikari had a reputation for being fast but sloppy. Apps R' Us was a smallish shop, so Steve was a good pick for taking over the project despite barely knowing the requirements.

Hikari was working on an augmented reality game for a very specific problem domain. In this use case, GPS was going to be unreliable. Instead, they needed to focus on the compass heading and rough location most times, at most correcting a little from GPS data. Steve skimmed through the code, looking for the overall structure before he dove into the fine details, but the following comment stopped him in his tracks:


   // The bluetooth manager and location manager must be initialized at app launch
   // There must be a delay between initialisation and checking whether bluetooth/location
   // services are available as the manager will ALWAYS say that it is unavailble at the point
   // of initialisation as it hasn't yet had chance to properly determine the status

Huh? Steve thought to himself. Weird. Why do I have to handle that in the app code? Shouldn't the manager class have a state machine that can check on this? And why are there so many typos anyway?

He moved to the LocationManager to take a look at what that code was doing so he could understand what made a state machine a poor choice. And that's when he found this particular beauty:


- (BOOL)_locationIsAccurate:(CLLocation *)location
{
    // The vertical and horizontal accurancy at its best tends to be at 3 and 5 respectively.
    // As such combining both values, dividing by 2 and checking if the value is <= 10,="" should="" be="" a="" good="" indicator="" that="" the="" person="" is="" outside="" with="" a="" good,="" clear="" signal,="" return="" (((location.horizontalaccuracy="" +="" location.verticalaccuracy)="" 2)=""><= 20);="" }="">

Problem one: the code doesn't match the comment; it compares the value to 20, not 10. Problem two: the comment makes no sense. What's going on? Are you subtracting the 3 from the 5 to get 2? Or is this an average? Assuming it's an average, why does the average being under 10 (or 20) mean "good signal?"

Wait ... good signal from the GPS? The GPS we're not using? Understanding dawned on Steve, and he checked for places the code was called.

The GPS was polled for location data, at best possible accuracy, once a second. The result, however, was checked for accuracy and then promptly disregarded on every single call.

There wasn't any use for this function at all.

Steve jumped away from his desk and swung over to his manager's office. "So, boss, I think I can save everyone's battery life in the next update ..."

[Advertisement] BuildMaster integrates with an ever-growing list of tools to automate and facilitate everything from continuous integration to database change scripts to production deployments. Interested? Learn more about BuildMaster!