Confusion with the `update` method of HandBrake::CLI

[bmatsuo]

This is suggestion based entirely on opinion. It is also probably fairly low priority due to the fact that the method in question is not critical to the functionality of the gem.

The update instance method for HandBrake::CLI will return true if the given HandBrakeCLI executable is up-to-date. I find this to be fairly counter-intuitive. I think the return value should be negated.

In my opinion, the contents of the conditional if hb.update then ... end should execute if the executable needs to be updated.

To avoid any confusion it might be good to replace hb.update with hb.update? or hb.updated? which, again in my opinion, are more descriptive and thus less ambiguous. The choice of name would depend on whether you want the method to return true when HandBrakeCLI is out of date (the former suggestion), or true when HandBrakeCLI is up-to-date (the latter).

However, changing the method name (especially to updated?) would break the current convention where method names translate directly to command line options (e.g. update translates to --update).

So I think it best to merely negate the return value of the existing method, as I first suggested.

[ghost]

Agree with the OP. But I think #updated? is the most ruby-appropriate solution.

alias :updated? :update

#update reads to me as if it will do the update process for you.

[bmatsuo]

Ahh alias. That looks good Yeah I'm pretty bad with ruby trickery. Sorry =P

But I think that's reasonable. The mere existence of the alias would clear up some ambiguity about the actual method's meaning.