Hi,
This is great. And it looks beautiful. Just, please make the labels gray by default :).
I reviewed the code a bit:
GET2Bar>>diagramClass
^ vertical
ifTrue: [ GET2VerticalBarDiagram ]
ifFalse: [ GET2HorizontalBarDiagram ]
GET2Bar>>horizontal
vertical := false
I think you should change two things. First, you could probably work directly with the diagram instance and not with the diagram class.
Second, instead of working with flags, just instantiate the correct object when you know it. For example:
GET2Bar>>horizontal
diagram := GET2HorizontalBarDiagram new
Also, could you please rename GET2AbstractBuilder to GET2Builder? (abstract does not add much to it)
In any case, I like it a lot that we have a RTBuilder hierarchy for everything related to visualizations. This should probably make it easy for creating fluent interfaces when wrapping in external interfaces like Glamour because the only thing we need to know from outside is to work with the interface from RTBuilder.
One downside for the current implementation of the builders is that there is no distinction between the specification step and the rendering step (they are done in one shot). This makes things simpler now, but it has implications:
- You cannot serialize the view specification object and render it in a different place (this for example, might be interesting for an Amber client and a Pharo server).
- You cannot perform optimizations in the builder or work with things that are not yet defined. For example, in a Mondrian builder the order of defining nodes and edges should not necessarily matter. This should be possible:
view edgesFrom: #superclass.
view nodes: classes.
This is only possible if we decouple the rendering from the specification.
I think it would be worth it for us to spend a bit of time looking at this, but we can also probably do it at a later time.
Doru