On Tue, Dec 28, 2010 at 07:46:15AM +0900, Jimmy C. wrote:
I’ve been working on a library that adds language translation methods to
strings using the Google Translate API. It’s my first time writing a
gem, using RSpec, and documenting with YARD. I’ve gotten it to a point I
feel is worth sharing but I’d love any feedback anyone can provide on my
specs, the documentation, or the code in general. Feel free to be harsh
if it’s bad, cause I’d like to learn and get better.
Mostly looks good. I’ve forked your project and pushed a few commits to
clean up (a little):
I’ll talk about each commit.
When you use
File.expand_path, you’re forcing that file to be in a
place relative to this file. That may seem OK, but what you’re really
saying is “I do not want ruby to consult the $LOAD_PATH when requiring”.
Where this really becomes a hindrance is if someone (even yourself) want
to provide an alternate implementation of say “to_lang/connector”, you
could just change the -I flags and provide the correct file. But with
expand_path form, you’ve prevented any hope of doing that.
Explicitly returning true or false usually isn’t a requirement. Very
rarely do people check that the return value of a method is equal to
true or false. Usually it’s used in a statement like this:
That statement doesn’t check for the “true” value, but an object that is
case / when statements use === when evaluating the
when part. You can
take advantage that Regexp#=== is the same as Regexp#=~ and reduce the
number of times you call
You should be able to use a similar technique in your method_missing
Overall, good job! Your code looks great! Keep hacking.