Eastwood rewriting

A discussion forum for the Eastwood Chart Servlet.
cblin
Posts: 12
Joined: Sun Jun 08, 2008 6:35 pm

Eastwood rewriting

Post by cblin » Mon Jun 09, 2008 1:53 pm

Hi,

we did a rewrite of eastwood :
* mavenize
* add unit tests before doing the refactoring (so that we can nearly guarantee that we did not change the behaviors)
* separate the 1800 lines into multiple classes (1 class per request parameter handling)
* rename the classes related to jfreechart (move them into a org.jfree.chart.{axis, labels, plot} )

we are ready to sign whatever agreement to give the code to jfreechart organization.

in the future, we plan to add image map capabilities (not part of gchart api), to add unit tests on each class to be able to test each component separately, to add chart types as we need them

regards,

chris

note : how do I upload an attachement so that you can have the zip file ?

Taqua
JFreeReport Project Leader
Posts: 698
Joined: Fri Mar 14, 2003 3:34 pm
Contact:

Post by Taqua » Mon Jun 09, 2008 2:08 pm

This forum does not allow uploads (you know, the evil spammers would make our life a hell and lawyers rich and happy).

Any patches should always be posted at
http://sourceforge.net/tracker/?group_i ... tid=315494
(preferrably with a notification here, to increase the visibility of the patch)

david.gilbert
JFreeChart Project Leader
Posts: 11734
Joined: Fri Mar 14, 2003 10:29 am
antibot: No, of course not.
Contact:

Re: Eastwood rewriting

Post by david.gilbert » Wed Jun 11, 2008 9:53 am

cblin wrote:we did a rewrite of eastwood :
Cool! Very cool!
cblin wrote: * mavenize
That dreaded word! I don't really understand Maven, although perhaps if I sat down and took the time to learn what it's all about then I'd become a convert. Maybe.
cblin wrote: * add unit tests before doing the refactoring (so that we can nearly guarantee that we did not change the behaviors)
I like unit tests!
cblin wrote: * separate the 1800 lines into multiple classes (1 class per request parameter handling)
In Subversion, I've just taken the parameter handling out of the servlet (because I want to use the parameter string to configure a chart in an applet using the Google Chart API, and also a Swing utility to make debugging a little easier...a very basic "proof of concept" of the latter just went into Subversion).

Did separate classes help? I'd have thought separate methods would have been sufficient. In spite of the fact that the parameters can be supplied in any order, there is a definite order in which they need to be processed when setting up the chart, and I wonder if that becomes obscured if you spread the code around multiple classes. But I guess I'll wait till I see what you've done.
cblin wrote: * rename the classes related to jfreechart (move them into a org.jfree.chart.{axis, labels, plot} )
Some of the customised axis and plot classes are sort of "crippled" to match the way that the Google Chart API works...which is why I didn't push those changes back into JFreeChart.
cblin wrote:we are ready to sign whatever agreement to give the code to jfreechart organization.
OK. I'm optimistic that we might be able to do away with the copyright assignment, but I'll get back to you about that.
cblin wrote:in the future, we plan to add image map capabilities (not part of gchart api), to add unit tests on each class to be able to test each component separately, to add chart types as we need them
Excellent. I've just started to do a little work on Eastwood again, and I'm keen to go in the direction of extending the Google Chart API to make it more useful. I think the overall concept is good, and the data encoding is a very good idea given the constraints, but there are some weaknesses that could be overcome (which is why I think you see quite a few people on the Google Chart API mailing list asking for the source code)..

With the work that Sun is doing to improve applet support, I'm also going to add an applet that can be driven off the same chart encoding (but, as an applet, it will be able to support tooltips, zooming etc.).
David Gilbert
JFreeChart Project Leader

:idea: Read my blog
:idea: Support JFree via the Github sponsorship program

cblin
Posts: 12
Joined: Sun Jun 08, 2008 6:35 pm

patch uploaded

Post by cblin » Mon Jun 16, 2008 1:45 pm

I've uploaded the patch on the tracker : ?func=detail&aid=1995094&group_id=15494&atid=315494

error : I can not post the link because of 'New and anonymous users are not allowed to have URLs in their postings.' whereas I am logged in

to answer your question :
* maven : the advantage of maven is that you can easily launch the tests, packages the war, ... simply by following directory conventions. If you do not like maven, I think it is very easy to create a ant file (given the few dependencies of eastwood)
* separating classes: it definitely helps to have 1 class per request parameter
1. when you have something wrong with the title, you look into GChartTitle class; something wrong with the data then you look into GChartData; ...
2. 1800 lines is really hard to understand (I know because that is what I felt the 1st time I see it)
3. processing is still done the way it was before
4. clearly separate what is jfreechart related and what is gchart related (I think that one day the gradient background can be part of jfreechart)
5. unit test each class separately (this is not done ATM)
* image map : wel'll did that this summer (probably)

regards,

cblin
Posts: 12
Joined: Sun Jun 08, 2008 6:35 pm

Patch accepted ?

Post by cblin » Thu Jun 19, 2008 8:39 pm

Hi,

I'd just like to know if my patch has been accepted by the eastwood team because I am going to start the implementation of the image map stuff so I am wondering if I can do it directly in the right source code

david.gilbert
JFreeChart Project Leader
Posts: 11734
Joined: Fri Mar 14, 2003 10:29 am
antibot: No, of course not.
Contact:

Post by david.gilbert » Tue Jun 24, 2008 3:33 pm

Hi Christophe,

Thanks. I see the patch now, you've posted it on the JFreeChart project page rather than the Eastwood project page. I'll transfer it across (having a busy week this week, so it may take a few more days before I can review it).

Regarding the image map, have you looked at how you might implement this yet? It most likely will require the chart to be generated twice, once for the HTML image map and then again for the PNG image...unless you want to store the chart image somewhere on the server between requests?

Regards,

Dave Gilbert

cblin
Posts: 12
Joined: Sun Jun 08, 2008 6:35 pm

Post by cblin » Tue Jun 24, 2008 3:40 pm

Hi david,

I added a request parameter named im in the format : id_html_map:SECTION1|SECTION2|...:drill_url

For ex, the PieChart Hello World :
im => "mapid:section1|section2:http://host/script.php?value=%s"

ATM, I've implemented it this way :

Code: Select all

in GChart class :
String imageMapStr = params.get("im");
/*...*/
if (imageMapStr != null) {
            TnxChartImageMap imageMap = new TnxChartImageMap();
            String im = imageMap.processImageMapStr(chartAndDatatype.chart, imageMapStr, dims[0], dims[1]);
            out.write(im.getBytes());
        } else {
            ChartUtilities.writeChartAsPNG(out, chartAndDatatype.chart, dims[0], dims[1]);
        }

in TnxChartImageMap class, I have :
public String processImageMapStr(JFreeChart chart, String imageMapStr, int width, int height) {
        String[] name_data_url = imageMapStr.split(":", 3);
        String imName = name_data_url[0];
        String[] imData = name_data_url[1].split("\\|");
        String imUrl = name_data_url[2];
        //TODO: is it really needed to render the chart in an image
        ChartRenderingInfo info = new ChartRenderingInfo();
        chart.createBufferedImage(width, height, info);
        Iterator<?> iter = info.getEntityCollection().iterator();
        int index = 0;
        while (iter.hasNext()) {
          ChartEntity entity = (ChartEntity) iter.next();
          String d = imData[index];
          index++;
          if (! "".equals(d)) {
              entity.setURLText(String.format(imUrl, d));
          }
        }
        return ImageMapUtilities.getImageMap(imName, info);
    }
I can submit a patch if you like.

Note : I'd like to know if this is the correct way to generate an image map in jfreechart

vbossica
Posts: 5
Joined: Sun Jun 29, 2008 10:26 pm

another similar implementation

Post by vbossica » Sun Jun 29, 2008 10:45 pm

hi all,

After looking at the Google API, I tested the idea of implementing the API using JFreeChart and add GeoTools (@ codehaus) for the map server. Everything would then be package into an independant WAR file for easy deployment.

I didn't go very far (my spare time is a scarce resource...) but had the following stack:

* maven build
* split the code into more classes
* spring + spring mvc backend
* commons-lang for handy methods.
* a couple of charts implemented (quite trivial I must admit)

Since Eastwood now passes my "business friendly license" test, I will for sure take a closer look at the effort and try to contribute to it if I can.

I will now play around with GeoTools and keep you posted (if you have any interest). Maybe this could be added to Eastwood to become an easy to use "charting server".

Best regards,

-Vladimir

david.gilbert
JFreeChart Project Leader
Posts: 11734
Joined: Fri Mar 14, 2003 10:29 am
antibot: No, of course not.
Contact:

Post by david.gilbert » Tue Jul 01, 2008 8:09 am

cblin wrote:I added a request parameter named im in the format : id_html_map:SECTION1|SECTION2|...:drill_url
I think if you specify the URLs fully for each section, you'll run up against the maximum string length for the chart specification pretty quickly. Maybe it would be better to specify something that configures the generic format for a PieURLGenerator, and lets it generate URLs for each section.

I don't know of a good way to avoid generating the chart twice - you need to draw the chart in order to get the values for the image map, then you need (later) to draw it again when the request comes in for the PNG image. I know some developers have implemented schemes with JFreeChart where the first request (for the HTML image map) caches the chart image and returns that when the second request comes in...but I don't know enough about web programming to know how reliable or sensible this is.

Regards,

Dave
David Gilbert
JFreeChart Project Leader

:idea: Read my blog
:idea: Support JFree via the Github sponsorship program

david.gilbert
JFreeChart Project Leader
Posts: 11734
Joined: Fri Mar 14, 2003 10:29 am
antibot: No, of course not.
Contact:

Re: another similar implementation

Post by david.gilbert » Tue Jul 01, 2008 8:18 am

vbossica wrote:After looking at the Google API, I tested the idea of implementing the API using JFreeChart and add GeoTools (@ codehaus) for the map server.
I had a student working on a project to prototype an idea I had for implementing simple map support in JFreeChart (colour coded countries, states or regions) and the result looks promising. It would certainly be sufficient to implement the very basic mapping features in the Google Chart API. All I need right now is some freely resdistributable map data, then I'll add the new plot type to JFreeChart. Then we can avoid creating a dependency on GeoTools, which looks great but a little bit of overkill for Eastwood.

Regards,

Dave

vbossica
Posts: 5
Joined: Sun Jun 29, 2008 10:26 pm

Re: another similar implementation

Post by vbossica » Tue Jul 01, 2008 10:16 am

david.gilbert wrote:All I need right now is some freely resdistributable map data, then I'll add the new plot type to JFreeChart.
and also find a way to let developers create their own map data.

That's one of the reasons why I was interested in GeoTools. I completely agree that GeoTools looks overkill but the plan is to integrate several OSS projects (JFreeChart and GeoTools (?)), provide a dead simple installation (a simple war file) and hide the complexity behind an HTTP API.

Now, if JFreeChart offers the capability to work with maps and let developers add their own, it's certainly a very good thing.

Regards,

-Vladimir

cblin
Posts: 12
Joined: Sun Jun 08, 2008 6:35 pm

Post by cblin » Thu Jul 03, 2008 8:17 am

david.gilbert wrote: I think if you specify the URLs fully for each section, you'll run up against the maximum string length for the chart specification pretty quickly. Maybe it would be better to specify something that configures the generic format for a PieURLGenerator, and lets it generate URLs for each section.
Actually, this is what I did, sorry if I was not clear.
For ex, for a pie with a title and 2 sections, the im params looks like :

Code: Select all

map:title|L1|L2|1|2:http://host/script.php?id=%s
But I do not know if I use the URLGenerator correctly (see my post of Tue Jun 24)

Ok to generate the chart twice (not so bad and if I use a chart frequently I'll use a cache as this is the case for a GChart implementation because of the 50000 hits per day

Keep me updated about my patches (reorganization + image map) quickly because I am going to do some work on the image map in august and I'll need to know what to do with the eastwood library (either keep the original or do a fork, which I do not want because I'd prefer to commit to the original project)

regards,

chris

david.gilbert
JFreeChart Project Leader
Posts: 11734
Joined: Fri Mar 14, 2003 10:29 am
antibot: No, of course not.
Contact:

Post by david.gilbert » Fri Jul 18, 2008 3:50 pm

Hi Chris,

Sorry for the delay responding. I didn't apply your patch, some reasons are given here:

http://sourceforge.net/tracker/index.ph ... id=1022209

I hope you will take a look at the 1.1.0 release (which just went up today) and see if the refactoring in it makes the code clearer.

There is a Maven file in there now, too, which was contributed by another developer...I don't know if it is good or not, please let us know if it needs adapting.

I think for the image map, it probably makes sense to install a custom URL generator in the chart (after reading the image map arguments from the chart specification), rather than modifying the image map string after it has been generated.
David Gilbert
JFreeChart Project Leader

:idea: Read my blog
:idea: Support JFree via the Github sponsorship program

cblin
Posts: 12
Joined: Sun Jun 08, 2008 6:35 pm

Post by cblin » Mon Jul 21, 2008 9:01 am

Hi David,

Thank you for your answers

Firstly, I did not find the pom.xml in the 1.1.0 zip release so I can not comment on it...

Concerning the classes organization, the reasons you give are understandable so I'll respect them. I'll live with that.
Just a few points :
* dataType = 0, 1, 2; => private static final int CONSTANT could be used to improve the code clarity, moreover there are 3 and 4 used in the file and they are undocumented...
* removing the warnings concerning the serialVersionUid is quite simple and should be done to improve error localization
* a subpackage to identify what is jfreechart related could be done too (even if it is not named org.jfree.chart but org.jfree.eastwood.chart for example). ATM, it is quite hard for a beginner in jfreechart like me to locate where I could help and where I can not and also which class inherits from what.
* there should be 1 method by step : when you know that something has gone wrong in the axis, you could watch in the processAxis method (and there should be a method with only calls to submethods for beginners that are willing to understand the chart generation process). This is easy to do with IDE refactoring functionnalities...

Concerning the bitmap rendering being different on the different platforms, this is possible since I do not know the jvm internals about image handling and I only work under windows. So I'll live with that too.

Also, junit.jar is not available in the lib directory so that it is unclear which version should be used to run the tests (3.8.1 seems ok).
The tests of the ChartEngine are very lite though...
At least, I think you could take my EastwoodTest and remove the asserts so that it is not necessary to launch a servlet to test eastwood ...

concerning the image map, I do not get your point.
Installing an URL generator is what I did with the entity.setURLText, isn't it ?

Finally, I need feedback on the image map parameter : is the format "id_map|TITLE|LEGEND1|LEGEND2|...|SECTION1|SECTION2|...|drill_url" ok ?

cblin
Posts: 12
Joined: Sun Jun 08, 2008 6:35 pm

Post by cblin » Mon Aug 04, 2008 12:34 pm

I do not like to do that but : BUMP !

I am waiting for answers about my questions / recommendations, especially the 1 method by step and the dataType constant.

Locked