6 Replies Latest reply on Oct 3, 2013 2:39 PM by Brian Leathem

    Charts component - developer review?

    Matej Novotny Newbie

      Hello

       

      I was assigned a task to cover the brand new charts component with some JS tests.

      As I was struggling with the API I realized it might be nice to make a developer review of the code.

       

      So here is what I found troublesome about charts:

      1) Examples

      If I am about to write a test I need either documentation or some nice examples so I could somehow understand how to create a component and use it. As there is no full documentation I was scavenging for some examples - those are a part of the GitHub repo.

      What I found was in no way simple in comparison with, let's say, autocomplete(here) examples. There is a lot of commented out code, also the script parts could be written next to the component they belong to in order to increase readability. Or the scripts could be at least commented as to explain to which chart belongs which script.

      Some refactoring would help the cause I believe.

       

      2) Charts component initialization

      As you attempt to initialize a chart, there is a need to use something like this (taken from pie chart initialization example):

                                     handlers: {"eventFunction":function(event,eventType,seriesIndex,pointIndex,x,y){}} ,

                                      particularSeriesHandlers: {"onplotclick":[null],"onplothover":[null]},

                                      data: [[{"data":12500746,"label":"Service sector"},{"data":188217,"label":"Agricultural sector"},{"data":2995787,"label":"Industrial sector"}]],   //....


      Why do I need to set a handler if it is an empty function? The same goes for second attribute, particularSeriesHandlers, why do I need to set them to null?

      If its convinient to use these attributes sometimes, it may be better to make these null values default and the whole attribute then optional. In order to create some basic (eg. default) chart it should be enough to specify it's type (pie/line/...) and it's data (and whatever else cannot be defaultly empty or null).

       

      *) Suggestion

      This is merely a suggestion. Maybe we could think of some unified way for future JS components to make examples for them or create documentation (if examples were not enough). So that it would be easier to understand the component's API and then use it (eg. write tests).