Hi, On Jun 10, 2009, at 8:39 AM, Asiri Rathnayake wrote:
Hello Devs,
I have been examining Dan Miron's patches for http://jira.xwiki.org/jira/browse/XWIKI-3313 & http://jira.xwiki.org/jira/browse/XWIKI-2883 and they are working fine.
However I think there are few improvements / refactorings that should be done before comitting the code. Most importantly, I would like us to agree upon the xwiki-chart api and xwiki-chart-macro format.
xwiki-chart module ==============
In dan's implementation, the chart module is actually called xwiki-jfreechart because it is dependant on the jfreechart api. However I think we can extract a generic chart api from dan's implementation without much of a trouble. In his implementation there are three interfaces:
* ChartGenerator * DataSource * DataSourceFactory * PlotFactory
Out of these interfaces only PlotFactory is dependant on jfreechart.
IMO we can construct a generic chart api by making ChartGenerator & DataSource interfaces component roles, hiding PlotFactory in an internal package and getting rid of DataSourceFactory. So the final API would be something like:
interface ChartGenerator { File buildChart(DataSource dataSource, Map<String, Object> parameters, File outputDirectory) throws ChartGeneratorException; }
It should not return a File but an OutputStream instead. I'd call it generate rather than build. And remove outputDirectory. And mention it's generating an image. so: ChartImageGenerator.OutputStream generate(DataSource, Map);
And
interface DataSource { // Usual tablemodel like api (from dan's implementation) void init (params) // Initializes this datasource from the parameters passed in, no need of a separate factory. }
I don't understand why it doesn't have any method. init() doesn't seem a good method to have. A source needs to return something. Something like a Chart definition.
We can also go to a api / implementation approach with multiple modules as in:
/platform/core/trunk/xwiki-chart | |-> xwiki-chart-api | |-> xwiki-chart-jfree
But I don't know if this is worth the trouble.
It can be put in the same module for now. The jfreechart implementation will be in the internal package as component implementation.
xwiki-chart-macro =============
Need more time to think about this one. Thanks -Vincent
Currently the chart macro has a format like:
{{chart chartType="line" source="type:XDOM; table_number:1;range:A1-C3;series:rows" title="line (xy_line_and_shape, default)" width="640" height="480"/}}
I think we should change the "chartType" parameter to just "type" and the source parameter would be changed like:
source="<id>|<params>"
here we will use the <id> part as the component hint for the corresponding DataSource... and the <params> string can be anything specific for that DataSource (so we don't put any restrictions).
For an example, we could have a datasource named "xdom" like:
source="xdom| document = Main .WebHome ;table=someTableId;series=rows;hasHeaderRow=true;hasHeaderColumn=true"
We load the DataSource corresponding to "xdom" and let it initialize with the parameters string.... (that's why i put a init() method inside DataSource component role).
The rest of the parameters would be standard : title, height, width etc.
As a start we will have two types of DataSources: XDom and Inline (Already there).
This is a quite big discussion and I don't know If I have explained myself correctly. Please let me know what you think about this and how it can be improved.
Thanks a lot.
- Asiri