Rules of Thumb

Rule of Thumb #37: Know your method return types

Do you see anything wrong with this piece of code (aside from the fact that it’s completely out of context)?

Yeah, neither did I. It’s a method from the fantastic FriendsOfSymfony CommentBundle for Symfony2, and it cost me two hours of debugging.

See, the bundle allows for comments to be upvoted and downvoted, but so far there hasn’t been any form of rate-limiting merged into the master branch. One user can spam votes on a comment, completely negating the value of the metric. After a little bit of correspondence with one of the developers in a pull request for a different implementation of the same feature, I decided to create a validation constraint that would make sure a user’s vote was unique based on both user id and comment id.

The cookbook documentation is pretty clear, so it didn’t take long. I fired up a debug session, created a test comment, upvoted once, upvoted twice, confident that nothing would happen… and the upvote registered. Hmm.

I checked the database table; everything seemed to be in order, so I did a couple quick tests to be sure that my validator was failing properly. It was. Even did a quick var_dump(“foo”); die(); to make sure that the validator was actually being called during the validation process (I’m not proud). It was. I begrudgingly set some breakpoints and started stepping through the code line-by-line.

I’ll spare you the gory details, but at some point I realized that even though my ConstraintViolation was being added to the stack and the validator was telling me that it was failing, the code was still proceeding through the bundle’s vote creator as if the validator had passed. I eventually checked the Validator#validate() method and discovered that it wasn’t returning a true/false value as I’d expected: it was returning a ConstraintViolationList collection.

As it turns out, if (!$obj) { /* … / } is very different from if (!$bool) { / … */ }. Very different. Changed the one line of code from a ! logical operator to a count() method call, and the whole thing worked like a charm.

Rule of Thumb #37: Know your method return types. Would’ve saved me a few hours in running down code. On the upside, I now know a lot more about Symfony2’s constraint validation process!