From: Aaron Patterson <aaron@...>
Date: 2010-12-28T10:36:09+09:00
Subject: Re: to_lang: my first gem, looking for feedback

--UlVJffcvxoiEqYs2
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Dec 28, 2010 at 07:46:15AM +0900, Jimmy C. wrote:
> Greetings,
>=20
> 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):

  https://github.com/tenderlove/to_lang

I'll talk about each commit.

  https://github.com/tenderlove/to_lang/commit/f75bb7280795c0dbc2b990cf1c02=
0c25b8994cd8

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.

  https://github.com/tenderlove/to_lang/commit/1aacb779448d9886116a426825de=
a3049ad1e474

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".

  https://github.com/tenderlove/to_lang/commit/8eee042e27d1a12793b55ccedbef=
3f407ebe7e12

case / when statements use =3D=3D=3D when evaluating the `when` part.  You =
can
take advantage that Regexp#=3D=3D=3D is the same as Regexp#=3D~ 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.  :-)

--=20
Aaron Patterson
http://tenderlovemaking.com/

--UlVJffcvxoiEqYs2
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (Darwin)

iQEcBAEBAgAGBQJNGT79AAoJEJUxcLy0/6/GNdUH/AqRXI+qYGWQlu6eAztbWj+z
aAUXNi3rlYeP/C+bSlcdO1dtYnZuDJMs3LGRAu9rNil5QcWfCwFJiKiCRiFC7j9z
WwQeEVY6G/llfXLZiBFH4RXKtHMpDjAxVURiRXy5G06VxqNRerXilOCmsofUX9CP
8Yk5gvnZClQ0ZL+TS+FiCfxqdBn3YzAJSRzyMvjejdSXBFMM9D5/+TEwgs2YoCBV
tlQMWPLpmqIlJh3Cv1B1IQ9hewfttNSS6mbwBpHd+DmhOOEeC9XDV5aHcqBJfOpt
4LsnJnPlw5txcWTZDgE8gTlkP2dGbXDQ3WRwUIjvxiRZmCpIGlsKiQ5FW4ojYsQ=
=UVMW
-----END PGP SIGNATURE-----

--UlVJffcvxoiEqYs2--