-
Notifications
You must be signed in to change notification settings - Fork 31
API kou
互換性のために残していると思うけど消したい。
.newでレシーバーのクラス以外のオブジェクトが返ってくるのは紛らわしい。
charty/backends/にしているから。
Chartty::PlotterAdapter.inheritedで継承したら自動登録するよりも明示的にCharty::Backend.registerを呼んで登録してもらう方がいいんじゃないかな。自動登録にすると複数のバックエンド実装で共有したいアブストラクトバックエンドみたいなやつがあったときに隠せなくなっちゃう。
module Charty
class Gruff < Backend
register :gruff
end
endあと、どうせNameを定義しなければいけない(継承するだけの完全自動ではない)ならメソッドを呼んでもいいと思う。
NameでいくならせめてCharty::Backend::Name→Charty::Backend::NAMEというように全部大文字にした方がよいと思う。
RenderContextで全部の図をサポートするんじゃなくて、Chartというベースクラスを作って図毎にクラスを作った方がいいんじゃないかな。
module Charty
class Chart
end
class Scatter < Chart
def plot
@backend.scatter(...)
end
end
endこうすればバックエンドがcase whenしなくてもよくなる。オブジェクト指向言語ならcase whenはできるだけ避けたいところ。
RenderContextがあってもいいけど、ユーザーには見せたくない。RenderContextって本当にコンテキストかな。Dataだとプロット対象のデータと紛らわしい感じがするからSourceとかどうかな。Dataで#eachでデータを返して、ラベルとかはメタデータとしてData#x_labelで返すとかもいいかも。
Chart#@+を定義して図を合成できるようにしてもよさそう。
scatter + barPlotter経由じゃなく、直接オブジェクトを作ってもいいかも。
scatter = Charty::Scatter.new(data, ...)plotは描画で、renderはファイルに保存は紛らわしいから保存はsaveがいいと思う。
気持ちはわからなくもないけどinstance_evalはコンテキストがわかりにくくなるのでやめた方がよいと思う。せめて、ブロック引数でインスタンスを受け付けるようにしたい。
if block_given?
if block.arity >= 1
yield(configurator)
else
configurator.instance_eval(&block)
end
endbarとかだけで大丈夫。