lykaios/trysql — pull request #1 by sjungling (2012-03-30 22:03:23+00:00)
Title: Code Review: #1

Nate, you don't need to merge this pull-request. It can be used as a nice way of viewing diffs.

For the most part the code looks really good. I think you've got the code in all the right places in general. The few things I commented on are mostly optimization hints.

* `Dbq` doesn't need to be instantiated each time you want to load test data or you exec a query. As you can see from the diff, you just need to add `self.` to the beginning of a method in the `Dbq` model and then you can use w/o needing to `Dbq.new(:qtext=>foo)`
* Unlike Oracle, there's no character limit on variable names, I would try to be a little more descriptive in your variable names. The advantage here is that you'll be able to read your code aloud and grok what's happening.
* To wit, I always prefer `and` over `&&`
* In your controller you should move all of your public methods to the top. The easiest way to discern whether a method should be public vs. protected / private is where there is a corresponding view.
* I also added `` tags around your queries on the tutorial pages for readability.

Looking really good!