odyssey icon indicating copy to clipboard operation
odyssey copied to clipboard

Api overhaul, bump to v1.0.0

Open jethrodaniel opened this issue 4 years ago • 0 comments

There are a number of issues with the current API.

  • [x] formulas aren't namespaced. That is, require "odyssey" pollutes the global namespace with the formula classes.
  • [x] the shortcut way of calling a formula (Odyssey.formula_name) via method_missing has 2 problems: 1) method_missing is slow, and 2) this only works (currently) when calling a global formula class, i.e, it doesn't handle namespaced formula classes.
  • [ ] the arguments to Odyssey.analyze are unintuitive.
    • [ ] It's currently Odyssey.analyze(text, formula, all_stats) - the more idiomatic Ruby thing here to expect would be Odyssey.analyze(text, :formula => :Ari) (still supporting the Odyssey.analyze(text) short call with some default).
    • [x] The presence of all_stats is a code smell/indicator that it should be a separate method. Since we compute everything anyway, we may as well return all the data each time
  • [ ] The Engine class uses class variables everywhere - all that should be confined to instances of Engine objects - 1) OOP is easier if we limit the responsibility of state (so no class variables unless a good reason), and 2) shared class variables don't play well with threads.
  • [x] returning a hash for analyze's all_stats isn't the easiest API, and is harder to document. Let's return an Object instead

Most of this is just standard best practices. When I require "foo", I expect all the new stuff to be under the Foo namespace. I also want the API methods to be self-explanatory - analyze(text, ...opts) is easier to grok than analyze(text, nil, true).

Since the changes are so vast, I feel it's best to release them as v1.0.0.

I'll also add deprecations for the changes to a new v0.4.0 release.

planned API

  • Just one method, Odyssey.analyze
    • for :formulas, pass symbols for builtins, or formula instances
    • Odyssey.analyze(text, :formulas => [:Ari, MyFormula.new, :all]) (:all as a shortcut for all the builtins)
    • return value is an object that responds to methods for each of the stats
      • so o = Odyssey.analyze(text); o.score; o.sentences, etc..
      • this allows us to properly document the return structure as well
      • .to_h to return everything as a hash with symbol keys

We can still support the String refinement, but String#readability will just be a wrapper for analyze

jethrodaniel avatar Aug 05 '20 00:08 jethrodaniel