On Tue, Dec 28, 2010 at 07:46:15AM +0900, Jimmy C. wrote:
Greetings,
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):
GitHub - tenderlove/to_lang: A Ruby library that adds language translation methods to strings, backed by the Google Translate API.
I’ll talk about each commit.
do not use expand path when requiring. let $LOAD_PATH decide what is … · tenderlove/to_lang@f75bb72 · GitHub
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
the expand_path
form, you’ve prevented any hope of doing that.
returning `true` is only necessary if the caller expects that exact v… · tenderlove/to_lang@1aacb77 · GitHub
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:
if something.start(...)
...
end
That statement doesn’t check for the “true” value, but an object that is
“truthy”.
use case / when rather than calling to_s multiple times · tenderlove/to_lang@8eee042 · GitHub
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 method.to_s
.
You should be able to use a similar technique in your method_missing
method.
Overall, good job! Your code looks great! Keep hacking. 