#85 Chart2D's methods should have IAxis arguments, not AAxis

[Jason S]

The setAxis* functions take AAxis arguments. They should really take IAxis (or the refactored version of IAxis) arguments; it should be perfectly fine to pass in any instance of IAxis, not just one descended from AAxis.

I looked in the source code for Chart2D and it keeps a List<IAxis> so this should be fine for implementation; the question is whether it negatively impacts the interface for pre-existing application code, and I don't think it does.

[Achim Westermann]

Hi Jason,
when I did this I really was sad to do it this way. This is a big design question that might require some heavier refactoring. So I want to discuss this with you and anyone reading this.

The problem:
When an axis is added to the chart the axis itself has to get a reference on the chart it is added to. When you would try to implement a custom IAxis you would see that you need this reference and additionally the information on which side and dimension the axis serves the chart yourself (for: scaling, axis title painting,...) and then you would start to copy the code from AAxis to have a chart reference. Therefore there is this callback method "setChart(...)" in AAxis which opens the door to feed it with a wrong chart and other wrong arguments while it is only intended to the chart.

Solution 1 (which I did): enforce AAxis for adding the the chart.
Pro:
- AAxis has the setChart... method so the interface hides this.
- All possible custom specializations have to extend AAxis and inherit the information needed to do the work for the chart.
Con:
- AAxis is needed by chart (against the idiom to link to the interface in order to avoid implementation dependencies).
- By casting to AAxis you can still compromise the application by sending in wrong arguemnts.

Solution 2: Put "setChart(..)" to the interface.
Pro:
- No custom implementation has to extend AAxis.
- Chart links against interface with no dirty implementation dependencies.
Con:
- That method which is only intended as a callback for the chart is now even more public and still may be invoked everywhere with wrong arguments.
- Custom implementation have to re-work the fact that an axis needs a reference to the chart along with the dimension and side it serves for on the chart.

Solution 3: Kill the "addAxis..." methods, or at least deprecate them and add new "public IAxis createAxis<Dimension><Location>(...)" methods that internally will add the axis to the chart and always return an AAxis implanted with the chart and dimension/side information needed.

Con:
- API will not allow you to provide your custom IAxis implementation.

Pro:
- Chart only talks to IAxis officially (looks nice but as you cannot use your own IAxis it is not).
- API will not allow to assign a wrong chart/position/dimension to the axis implementations.
- Developers that implement a great improved IAxis will have to contribute this to the project or be screwed after next release... BIG GRIN towards the mentality to hack fast forward your project and only take but never give unless users first face the problems when their valuable extensions never contributed break with the API of a new release:-)

Any further solutions I did not find (as all solutions are not perfect)?

What do you think?

kind regards,

Achim

[Achim Westermann]

Forgot to say:

My personal vote would be:

Solution 1 for not breaking the API to previous versions and not forcing implementations to deal with the chart reference and position informations.

or

Solution 2: Put that dangerous method setChart(..) into the inferface, document that only the chart should invoke it and hope that no end user will mess with it.