Menu
▾
▴
#243 Dataset holder interface and new method 'setItemPaint'
1/ Following new interfaces: 'IDatasetHolder' and 'IMultipleDatasetHolder'. They try to gather Plot instances that have methods like 'setDataset' and 'getDataset'. XYPlot, CategoryPlot and PiePlot have been updated (other needed?).
2/ Following new method: 'setItemPaint' in 'AbstractRenderer' and 'XYItemRenderer'.
1/ Sometimes, when a class does not know which plot is used, IDatasetHolder resp. IMultipleDatasetHolder is useful to access/read/write the Dataset currently used. This approach is preferable to using 'instanceof'. However I changed the 'IDatasetHolder.getDataset' method name to 'IDatasetHolder.getPlotDataset' in the second path I submitted. 'IDatasetHolder.getDataset' would work with Java 1.5 but not with Java 1.4 without using cast.
2/ I do not understand why there is not setter for 'itemPaint'. There is a getter 'getItemPaint' and there is a setter for 'seriedPaint' for instance.
Second patch submitted
Thanks for the patches.
For the dataset holder interfaces, I'd like to understand better the motivation behind the change. Certainly I think there is a cost (particularly for new users to the API) in returning just a Dataset instead of the more specific PieDataset, CategoryDataset and XYDataset. When you say that this approach "is preferable to using 'instanceof'", what is preferable? Just that it makes your code tidier/cleaner? Or is there more to it? I'm actually thinking that an alternative implementation might just be some static methods in ChartUtilities:
public static Dataset getDataset(JFreeChart chart);
public static int getDatasetCount(JFreeChart chart);
public static Dataset getDataset(int index, JFreeChart chart);
For the setItemPaint() change, I have been thinking of adding this feature. Your change looks fine, except that the use of XYDataItem as the key for the maps might be slightly wasteful---you could make a smaller dedicated key class based on integers---and adding a method to the XYItemRenderer isn't a backwards compatible change.
Dave Gilbert
JFreeChart Project Leader
Patch for only adding 'setItemPaint' method to 'AbstractRenderer'
Hi Dave,
I extracted a 'set_item_paint.patch' out of 'jfreechart.patch' (see below). As suggested I made a smaller dedicated key class (I was anyway not happy with the use of a class located in another package). I let my IDE (Eclipse) compute the 'serialVersionUID'. As I am using JDK1.5, I am not sure you get the same number when using JDK1.3. I also extended 'AbstractRendererTests' class with an appropriate test.
> adding a method to the 'XYItemRenderer' isn't a backwards compatible change.
This is true. I do not know what is the policy currently applied in the JFreeChart project. People implementing their own 'XYItemRenderer' from the scratch without extending 'AbstractRenderer' will effectively get a compiler error. This is probably a minority (if any). I would say this is acceptable if mentioned in the change log. But it is your decision.
Hi Dave,
> For the dataset holder interfaces, I'd like to understand better the motivation behind the change.
> Certainly I think there is a cost (particularly for new users to the API) in returning just a Dataset
> instead of the more specific PieDataset, CategoryDataset and XYDataset.
You might be right. I added 'getPlotDataset' to the interface because it seemed natural to me (actually, with JDK1.5, you would get the right subtype). Actually, I am more concerned about setting the 'Dataset'. My situation is the following: I programmatically try to set a new 'Dataset' without creating a new 'JFreeChart' and I do not know which kind of plot I get/have at that point. So either the plot implements an interface which sets the new 'Dataset' for me. Or I have to go over all the plot subclasses the library has and then I have to call the appropriate method on the plot found for setting the new dataset (using 'instanceof'). Did you get me? What do you think?