Help save net neutrality! Learn more.
Close

#847 Discrepancy between CombinedDomain*Plot classes

closed-fixed
General (896)
7
2008-03-31
2008-03-26
No

I am reworking my combined domain and range plot classes (see https://sourceforge.net/tracker/?func=detail&atid=315494&aid=1924543&group_id=15494 (patch 1924543)), and I have noticed a discrepancy between the CombinedDomainCategoryPlot and CombinedDomainXYPlot. In the latter, the getDataRange(ValueAxis) method has been defined to return the combined rnage of all the subplots. In the former, the getDataRange(ValueAxis) method is not defined, so the CombinedDomainCategoryPlot relies on the CategoryPlot class' version. The result is that calling getDataRange on a CombinedDomainCategoryPlot returns a default axis range of [0.0, 1.05].

This discrepancy can be fixed by either adding CombinedDomainXYPlot's getDataRange(ValueAxis), appropriately modified, to CombinedDomainCategoryPlot or remove the override from CombinedDomainXYPlot. While I do not have strong feelings as to which course of action should be taken, I do feel strongly that this should be fixed. The argument for the former solution is backwards compatibility in case people have relied on CombinedDomainXYPlot's getDataRange(ValueAxis). The argument for the latter solution is 'correctness' since the combined plot does not really have a data range per se.

I will be updating the patch with my combined domain and range plot classes, and I will be noting this bug in the comments.

Discussion

  • Richard West

    Richard West - 2008-03-26
    • priority: 5 --> 7
    • assigned_to: nobody --> mungady
     
  • David Gilbert

    David Gilbert - 2008-03-27
    • labels: --> General
    • status: open --> closed
     
  • David Gilbert

    David Gilbert - 2008-03-27

    Logged In: YES
    user_id=112975
    Originator: NO

    Hi Richard,

    I think the code is correct, although not very clear. First, these methods are only public so they can be accessed by the axis classes that need to determine their 'auto range'. It isn't really intended that these methods be called by user code. Second, although the method signature is designed so that it can be called for either a domain axis or a range axis, in the case of the CombinedDomainXYPlot it will only ever be called for a domain axis (because, as you spotted, it doesn't really make sense to find the data range for the subplots against the range axes since these are all independent). That's why, for the CombinedDomainCategoryPlot, the method was never overridden (it's unnecessary, because the shared domain axis is a CategoryAxis, which has no 'auto range' to calculate...there may well be bugs relating to aggregating the categories from the subplots, but that's another issue).

    So to "fix" this bug, I've committed some additional javadoc comments that perhaps make this clearer. I was also tempted to throw some exceptions if the methods were called with an invalid axis, but maybe that would just be an annoyance for anyone who is already using these methods.

    Sorry for the confusing code...I hope I've explained it well enough.

    Regards,

    Dave

     
  • Richard West

    Richard West - 2008-03-27

    Logged In: YES
    user_id=1788593
    Originator: YES

    I guess this makes sense. For consistency, I shall have both CombinedCategoryPlot and CombinedXYPlot (see the aforementioned patch) override the getDataRange method. This will prevent problems if the CommonDomain*Plot classes ever start inforcing that only the domain axis can call the getDataRange method.

     
  • Richard West

    Richard West - 2008-03-27

    Logged In: YES
    user_id=1788593
    Originator: YES

    I am encountering a slight problem when trying to use getSubplots in CombinedXYPlot's redefine of CombinedDomainXYPlot's getDataRange(ValueAxis). Since getSubplots uses Collections.unmodifiedList(this.subplots), a NullPointerException is thrown if this.subplots is null. getSubplots should be redefined to either return null if this.subplots is null otherwise return Collections.unmodifiedList(this.subplots).

     
  • Richard West

    Richard West - 2008-03-27

    Logged In: YES
    user_id=1788593
    Originator: YES

    addendum: or have getSubplots return an empty List if this.subplots is null.

     
  • Richard West

    Richard West - 2008-03-29
    • status: closed --> open
     
  • Richard West

    Richard West - 2008-03-29

    Logged In: YES
    user_id=1788593
    Originator: YES

    It took me a while, but I figured out what was causing the NullPointerException in my overridden getSubplots(). CombinedXYPlot's constructor calls CombinedDomainXYPlot's constructor. CombinedDomainXYPlot's constructor initializes this.subplots to an empty ArrayList, but it does so only after calling XYPlot's constructor. XYPlot's constructor sets the domain axis and tries to autorange the axis. As part of the autoranging, the axis calls getDataRange for CombinedXYPlot which in turn calls getSubplots(). getSubplots tries to return an unmodified list from the still undefined this.subplots and results in the NullPointerException. The relavent (slightly redacted) backtrace is below.

    Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
    at java.util.Collections$UnmodifiableCollection.<init>(Collections.java:994)
    at java.util.Collections$UnmodifiableList.<init>(Collections.java:1148)
    at java.util.Collections.unmodifiableList(Collections.java:1134)
    at org.jfree.chart.plot.CombinedDomainXYPlot.getSubplots(CombinedDomainXYPlot.java:327) // relies on this.subplot which has yet to init
    at <redacted>.CombinedXYPlot.getDataRange(CombinedXYPlot.java:45)
    at org.jfree.chart.axis.NumberAxis.autoAdjustRange(NumberAxis.java:426)
    at org.jfree.chart.axis.NumberAxis.configure(NumberAxis.java:409)
    at org.jfree.chart.axis.Axis.setPlot(Axis.java:821) // domain axis configured as part of XYPlot constructor
    at org.jfree.chart.plot.XYPlot.<init>(XYPlot.java:551) // super
    at org.jfree.chart.plot.CombinedDomainXYPlot.<init>(CombinedDomainXYPlot.java:154) // super followed by this.subplot init
    at <redacted>.CombinedXYPlot.<init>(CombinedXYPlot.java:16) // constructor

    The reason why getRangeAxis in CombinedDomainXYPlot does not fail is because it guards against this.subplot == null. this.subplots is private (which it should be), so CombinedXYPlot does not have the liberty to check if it is null. I propose that getSubplots() returns an empty ArrayList if this.subplots is null.

    In terms of CombinedXYPlot: I can either have it override getDataRange to hedge against future enforcement of which axes can use it, or I could simply leave out the getDataRange override since CombinedDomainXYPlot's handles both the domain and the range.

     
  • David Gilbert

    David Gilbert - 2008-03-31

    Logged In: YES
    user_id=112975
    Originator: NO

    Thanks Richard. I committed the change to getSubplots() so it returns Collections.EMPTY_LIST if this.subplots is null (for inclusion in the 1.0.10 release, which I'll try to get out this week).

    Regards,

    Dave

     
  • David Gilbert

    David Gilbert - 2008-03-31
    • status: open --> closed-fixed
     

Log in to post a comment.