Multiple Chart Series issue

[mouse]

Hi, Cagatay, and other developers and users,

While still having this issue - http://primefaces.prime.com.tr/forum/vi ... 48&start=0 - I'm continuing work with the charts to get it working for me. I've discovered what I think is a bug. This is in the JSF 1.2-based version.

Here is a code segment:

Code: Select all

<p:lineChart value="#{searchBean.searchResult.rows}" var="row" xfield="#{row.xaxis}">
	<c:forEach var="fieldName" items="#{searchBean.searchResult.fieldNames}">
		<p:chartSeries label=" #{fieldName}" value="#{row.values[fieldName]}"/>
	</c:forEach>
</p:lineChart>

I'm noticing that all of the generated JavaScript variables are called "fieldName".

Code: Select all

YAHOO.widget.Chart.SWFURL = '/SMSWeb/primefaces_resource/1.0.2/yui/charts/assets/charts.swf'</script><div id="mainform:j_id89" style="width:500px;height:350px"></div><script type="text/javascript">jQuery(document).ready(function(){widget_mainform_j_id89_data = [{xaxis:"2006",fieldName:15.0,fieldName:6.0,fieldName:43.0,fieldName:0.0,fieldName:0.0,fieldName:0.0,fieldName:0.0},
{xaxis:"2007",fieldName:23.0,fieldName:13.0,fieldName:19.0,fieldName:1.0,fieldName:1.0,fieldName:0.0,fieldName:0.0},
{xaxis:"2008",fieldName:26.0,fieldName:17.0,fieldName:53.0,fieldName:7.0,fieldName:0.0,fieldName:3.0,fieldName:0.0},
{xaxis:"2009",fieldName:23.0,fieldName:8.0,fieldName:38.0,fieldName:3.0,fieldName:0.0,fieldName:1.0,fieldName:1.0},
{xaxis:"2010",fieldName:3.0,fieldName:1.0,fieldName:14.0,fieldName:1.0,fieldName:0.0,fieldName:0.0,fieldName:0.0}];
var widget_mainform_j_id89_dataSource = new YAHOO.util.DataSource(widget_mainform_j_id89_data);
widget_mainform_j_id89_dataSource.responseType=YAHOO.util.DataSource.TYPE_JSARRAY;
widget_mainform_j_id89_dataSource.responseSchema = {fields:["xaxis","fieldName","fieldName","fieldName","fieldName","fieldName","fieldName","fieldName"]};
var widget_mainform_j_id89_seriesDef = [{displayName:' B727', yField:'fieldName'},{displayName:' CB737', yField:'fieldName'},{displayName:' [none]', yField:'fieldName'},{displayName:' B747', yField:'fieldName'},{displayName:' B757', yField:'fieldName'},{displayName:' B767', yField:'fieldName'},{displayName:' B777', yField:'fieldName'}];
var yAxis = new YAHOO.widget.NumericAxis();
var xAxis = new YAHOO.widget.CategoryAxis();
widget_mainform_j_id89 = new YAHOO.widget.LineChart('mainform:j_id89',widget_mainform_j_id89_dataSource,{xField:'xaxis',series:widget_mainform_j_id89_seriesDef,expressInstall:'/SMSWeb/primefaces_resource/1.0.2/yui/assets/expressinstall.swf',xAxis:xAxis,yAxis:yAxis});
});

LineChartRenderer.encodeLocalData() has some lines that look like this:

Code: Select all

			for (ChartSeries axis : series) {
				ValueExpression ve = axis.getValueExpression("value");
				String fieldName = getFieldName(axis.getValueExpression("value"));
				
				writer.write("," + fieldName + ":" + ve.getValue(facesContext.getELContext()).toString());	//TODO: Use converter if any
			}

The setting of the fieldName variable is a little dodgy. The sample lineChart in the manual has two explicit chartSeries, with values of "#{birth.boys}" and "#{birth.girls}". The getFieldName() expression takes that value expression, and parses out the "boys" or "girls" and uses that for the variable name in JavaScript. There is similar handling in the encodeSeriesDef() method.

Now getting back to my example. For me, p:chartSeries is in a loop. It takes the "value" parameter, which in my case is "#{row.values[fieldName]}". It cuts out "fieldName" and uses that as the variable name ... for ALL rows! Since all variables in a row have the name "fieldName", the last one wins, and there is only one data series drawn.

Can there be another way of setting the field name? "label" is nice for a user displayed value, but there needs to be something else for the generated JavaScript variable name.

I thought of two possibilities to change the behavior.

SUGGESTION #1: add an attribute to specify the JavaScript variable name:
<p:chartSeries label="" value="" variable=""/>
If the variable weren't specified, it would default back to the current behavior, which would retain compatibility. You would probably have to change all of the chart objects to support the new attribute.

SUGGESTION #2: change the algorithm that determines the variable name. Maybe make it based on the label, to lowercase, spaces to _, minus / dash ("-") to _, and maybe other special characters to underscore. Code written for the old version *SHOULD* still work, although there might be some weird cases where it wouldn't. It would probably only involve changing the getFieldName() method, although it would have to pass in the label as well as the value.

ETA: SUGGESTION #3: Forget reasonable names for variables, just make them "field1", "field2", "field3", etc. This is probably the most reliable for users of the components.

Regards,
Chris

[callahan]

Hi Chris,

I'll have to agree with you. The getFieldName method in BaseChartRenderer is a bit on the dodgy side. It more or less makes it impossible to use c:forEach to generate the list of p:chartSeries for a chart dynamically. You could however resort to the p:lineCharts binding attribute and generate the charts list of p:chartSeries components in a backing bean.

[cagatay.civici]

Can you please create an issue about this so we don't lose this discussion, in 1.1.1 and 2.1.1 we can work on to improve this. Dynamic chart series has been in the plans for some time.

[mouse]

I have created a bug for this:

http://code.google.com/p/primefaces/iss ... ail?id=965

[mouse]

bmacmanus said:

You could however resort to the p:lineCharts binding attribute and generate the charts list of p:chartSeries components in a backing bean.

I'm not even sure that would help. The problem exists in the rendering of the component. It still checks the value expression and evaluates it. I do not have a case where the expression is of the form #{birth.girls} where that expression is used to come up with the javascript variable name "girls". The value expression is used both to retrieve the data (which works fine) and to come up with a JavaScript variable name. Therefore my expression #{row.values[fieldName]} uses "fieldName" for every column's javascript variable name. I don't think it helps whether the expression is set in the Facelets markup or in code, the problem is at render time. (Correct me if that analysis is wrong.)

[mouse]

I'm trying to compile so I can submit a patch for this problem. When I run, there are no errors, but the markup just passes through to the final HTML. For example, I get <p:resources/> in my output. This is the same code that was working when I used the distribution JAR. I'm running Tomcat 6.0.24 within Eclipse. The primefaces-1.0.2.jar appears in the tmp folder used to run the application, with today's date (indicating it was built from the Eclipse project) and all the class files with today's date and all of the resources are inside the JAR file.

I tried doing searches through the forum and Google, but the possible search terms I can come up with are either too generic where I get all irrelevant results or too specific to where I get no results.

Thanks,
Chris

[mouse]

Found it. The Sources JAR does no include important files in META-INF, including:
faces-config.xml, primefaces-i.taglib.xml, primefaces-i.tld, primefaces-p.taglib.xml, primefaces-p.tld

So now I am able to verify that my fixes work correctly. I'll submit a patch to the bug case.
http://code.google.com/p/primefaces/iss ... ail?id=965

[mouse]

I have posted an updated version of LineChartRenderer.java to the bug report. The other chart component files will need to be similarly modified, but the changes are relatively simple. I'm also not sure whether you use the same code base for 1.0.2 and 2.0.2, but hopefully the change will be applied there, as well.

http://code.google.com/p/primefaces/iss ... ail?id=965

[callahan]

I see that you've sorted your problem out and don't need the binding attribute. I couldn't resist trying it out though. The following works:

The page:

Code: Select all

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html xmlns="http://www.w3.org/1999/xhtml"
      xmlns:h="http://java.sun.com/jsf/html"
      xmlns:p="http://primefaces.prime.com.tr/ui"
      xmlns:c="http://java.sun.com/jsp/jstl/core"
      xmlns:ui="http://java.sun.com/jsf/facelets"
      >

<h:head>
</h:head>

<h:body>
  <h:form>

    <p:lineChart value="#{lineChartBean.births}" var="birth" xfield="#{birth.year}" binding="#{lineChartBean.lineChart}">
    </p:lineChart>

  </h:form>
</h:body>

</html>

Birth2 - A modified Birth class:

Code: Select all

package controller;

import java.io.Serializable;
import java.util.HashMap;

public class Birth2 implements Serializable {
    private int _year;
    private HashMap<String, Integer> _data;

    public Birth2(int year, HashMap<String, Integer> data) {
        _year = year;
        _data = data;
    }

    public int getYear() {
        return _year;
    }

    public HashMap<String, Integer> getData() {
        return _data;
    }
}

The backing bean:

Code: Select all

package controller;

import java.io.Serializable;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Set;

import javax.el.ExpressionFactory;
import javax.el.ValueExpression;

import javax.faces.bean.ManagedBean;
import javax.faces.bean.RequestScoped;

import javax.faces.context.FacesContext;

import org.primefaces.component.chart.line.LineChart;
import org.primefaces.component.chart.series.ChartSeries;

@ManagedBean
@RequestScoped
public class LineChartBean implements Serializable {
    private List<Birth2> births;
    private LineChart lineChart;

    public LineChartBean() {
        births = new ArrayList<Birth2>();

        HashMap<String, Integer> data = new HashMap<String, Integer>();
        data.put("boys", 120);
        data.put("girls", 52);
        data.put("men", 10);
        data.put("women", 50);
        births.add(new Birth2(2004, data));

        data = new HashMap<String, Integer>();
        data.put("boys", 100);
        data.put("girls", 60);
        data.put("men", 20);
        data.put("women", 40);
        births.add(new Birth2(2005, data));

        data = new HashMap<String, Integer>();
        data.put("boys", 44);
        data.put("girls", 110);
        data.put("men", 30);
        data.put("women", 30);
        births.add(new Birth2(2006, data));

        data = new HashMap<String, Integer>();
        data.put("boys", 150);
        data.put("girls", 135);
        data.put("men", 40);
        data.put("women", 20);
        births.add(new Birth2(2007, data));

        data = new HashMap<String, Integer>();
        data.put("boys", 125);
        data.put("girls", 120);
        data.put("men", 50);
        data.put("women", 10);
        births.add(new Birth2(2008, data));
    }

    public List<Birth2> getBirths() {
        return births;
    }

    public LineChart getLineChart() {
        return lineChart;
    }

    public void setLineChart(LineChart lineChart) {
        if (this.lineChart == null) {
            FacesContext fc = FacesContext.getCurrentInstance();
            ExpressionFactory ef = fc.getApplication().getExpressionFactory();

            for (String label : getLabels()) {
                ChartSeries cs = new ChartSeries();

                ValueExpression labelExpr = ef.createValueExpression(fc.getELContext(), label, String.class);
                ValueExpression valueExpr = ef.createValueExpression(fc.getELContext(),"#{birth.data." + label + "}", int.class);

                cs.setValueExpression("label", labelExpr);
                cs.setValueExpression("value", valueExpr);
                lineChart.getChildren().add(cs);
            }
        }

        this.lineChart = lineChart;
    }

    public Set<String> getLabels() {
        // This is a bit on the dodgy side.
        return getBirths().get(0).getData().keySet();
    }
}

[mouse]

BMacManus,

Yes, very clever! I think what you wrote will work, although I don't think I could have figured it out on my own because I don't yet understand ValueExpressions well enough. I believe it would not work if you wanted to graph each line as a department, such as "Daily Operations", "Accounting", "Information Technology"; or as "Boys 5-9", "Boys 10-14", "Boys 15-19", "Girls 5-9", "Girls 10-14", "Girls 15-19"; or names "Bob Parker", "Emily Baker", "Anil Thambu". (Names and Departments are actual cases I have.) I think that would break both the ValueExpression and the JavaScript that is to be generated. You might be able to fix the value expression by using something like this:

Code: Select all

ValueExpression valueExpr = ef.createValueExpression(fc.getELContext(),"#{birth.data['" + label + "']}", int.class); // notice single quotes

But the generated JavaScript variable name would still be broken.

I hope the committers will like the changes I made for LIneChartRenderer and make the corresponding changes for the other chart components (which are neither difficult nor time consuming).