Check if objects have coordinates

Question

Here's a function:

func checkIfObjectsHaveCoordinates() {
    PFQuery(className: "Products").findObjectsInBackgroundWithBlock {
        (objects: [PFObject]?, error: NSError?) -> in
        if error == nil && objects != nil {
            for object in objects! {
                if object["Latitude"] == nil || object["Longitude"] == nil {
                    self.convertObjectLocationToCoordinates(object)
                }
                object.pinInBackground()
            }
        } else {
            print(error)
        }
}

It checks if objects have a latitude or a longitude. If they don't, they will be passed to the convertObjectLocationToCoordinates function. Either way, I save them locally, and this is why I think my function should't have side effects. I feel like this code is hardcoded in some way:

if myObject["Latitude"] == nil || myObject["Longitude"] == nil || myObject["Latitude"] as! Float == 0 || myObject["Longitude"] as! Float == 0

Does this function look good to you? Should I avoid side effects in my function?

Answer 1

What constitutes an effect and what constitutes a side-effect may be a matter of framing expectations. The real problem, I think, is that the function name fails to convey what it actually does. A more appropriate name might be something like pinProductsInBackgroundWithCoordinates(), because:

• "check if objects have coordinates" only vaguely describes what this function is doing. What if it does have coordinates — then what? And if it doesn't, then what?
• If the query succeeds, then wouldn't all of the objects end up having coordinates?
• It looks like .pinInBackground() is going to be called for all of the objects.

If you take my naming suggestion, then pinning products is the effect, and ensuring that they all have coordinates is the side-effect. That's much better than checkIfObjectsHaveCoordinates(), in which case both actions would be considered side-effects.

---

I'm not quite comfortable with the error handling. I think that rearranging it would clarify both branches of the conditional.

if error != nil {
    print(error)
} else if objects != nil {
    for object in objects! {
        …
    }
}