abacha/s10-mod — Issue #12: Issues with bin/risk_calculator.rb
Author: sandal
Created: 2012-01-22 02:49:11+00:00
State: open

1) Rather than putting most of your code in your script, you should create a `RiskCalculator::Application` class in _lib/risk_calculator/application.rb_ which encapsulates your logic. While the advice in [this Practicing Ruby article](http://practicingruby.com/articles/9) is probably more than what you need, you can get the idea of the basic structure from skimming through it and looking at how bin/rcat interacts with `RCat::Application`

2) Rather than using `require "highline/import"`, I'd recommend using `require "highline"` and then doing something lik:

```ruby
io = HighLine.new
io.menu do
#...
end
```

This will prevent your application from causing HighLine to pollute the global namespace.

3) Rather than calling this file `bin/risk_calculator.rb` it should be called `bin/risk_calculator` and use a shebang line to indicate it's a Ruby executable. This makes it possible to run the command in your shell as `risk_calculator` instead of `risk_calculator.rb`, which will make it feel more like an ordinary UNIX command.

4) Instead of inheriting from `Exception`, it's better to inherit from `StandardError`. This is because `StandardError` is the class Ruby provides for use as a parent class for custom exceptions, is what a bare rescue matches. The kinds of exceptions which inherit from `Exception` typically are things that can't be rescued in a general case, such as syntax errors.

5) While it's optional, I'd encourage you to write `InvalidParameterException = Class.new(StandardError)` instead of defining a bare class definition, simply because it's a bit more aesthetically pleasing and is functionally equivalent.

6) To get the full benefits of HighLine, it's best to use it for all your console IO needs. That means replacing calls to `puts` with calls to `HighLine#say`.

7) Instead of `params = _params.split(';')`, consider using a more intuitive name for _params, such as `raw_params` or `params_text`, yielding something like `params = params_text.split(';')`

8) Please remove any unused code rather than leaving it commented out in the source.