2
\$\begingroup\$

I have written a code to generate chart data for the Highcharts library and need to know if there are any improvements possible. I have applied generics but think it could be fine tuned further. Could somebody recommend anything?

I have basically written a GenerateBoxPlotChartSeries method that would generate BoxPlotChartSeries as the name suggests.

There are two methods that have the same name accepting different parameters. Is it a good idea to have two different methods or is it better to have a common method and apply generics to the parameter and then check based on the parameter and execute the if else condtion within the method? My only worry is that the length of the method would go on increasing. Also note that I would be creating more graphs like Line Chart and Spline Chart. The classes and methods need to take this into consideration as well. So the method will also need to Generate spline chart data and line chart data accordingly

Please note the following

  1. NpvGraphs would contain CommIns,CaptiveIns,SelfIns and EVAGraphs would contain CaptiveView and ParentView.

  2. I have defined BloxPlotSeries data for BloxPlot chart. Similarly I would need to define additional classes for spline chart and line chart. Assume they have different properties while suggesting better coding standards in your answer.

I would be soon incorporating other charts apart from boxplotchart. How do I handle that?

public class Series<T>
{
    public List<T> data { get; set; }
}

public class BloxPlotSeriesData
{
    [JsonMinify]
    public double low { get; set; }

    [JsonMinify]
    public double q1 { get; set; }

    [JsonMinify]
    public double median { get; set; }

    [JsonMinify]
    public double q3 { get; set; }

    [JsonMinify]
    public double high { get; set; }

    [JsonMinify]
    public string Color { get; set; }
}

public class NPVGraphData
{
    [JsonMinify]
    public List<double> CommIns { get; set; }
    [JsonMinify]
    public List<double> CaptiveIns { get; set; }
    [JsonMinify]
    public List<double> SelfIns { get; set; }
}

public class EVAGraphData
{
    [JsonMinify]
    public List<double> CaptiveView { get; set; }
    [JsonMinify]
    public List<double> ParentView { get; set; }
}

Calls to the method

NpvResult = new Models.NpvResults()
{
  SeriesData = GenerateBoxPlotChartSeries(trigger2Output.NpvResults.GraphData)

}


 EvaResult = new Models.EvaResults()
 {
 SeriesData =GenerateBoxPlotChartSeries(trigger2Output.EvaResults.GraphData)
}

Similarly I am going to have two more calls to the method that will generate spline chart and line chart accordingly. The method call above only does for BoxPlotChart.

StressResult = new Models.StressTestAnalysis()
{
SeriesData=GenerateSplineChartSeries(trigger2Output.StressTestAnalysis.GraphData)
}

    EndingSurplus = new EndingSurplus()
    {
      SeriesData=GenerateLineChartSeries(trigger2Output.EndingSurplus.GraphData)
    } 

Methods

private Series<BloxPlotSeriesData> GenerateBoxPlotChartSeries(EVAGraphData evaResultsGraphData)
{

    //CaptiveView
    var captiveViewSeriesData = new BloxPlotSeriesData
    {
        low = evaResultsGraphData.CaptiveView[0],
        q1 = evaResultsGraphData.CaptiveView[1],
        median = evaResultsGraphData.CaptiveView[2],
        q3 = evaResultsGraphData.CaptiveView[3],
        high = evaResultsGraphData.CaptiveView[4],
        Color = "#C111A0"

    };

    //ParentView
    var parentViewSeriesData = new BloxPlotSeriesData
    {
        low = evaResultsGraphData.ParentView[0],
        q1 = evaResultsGraphData.ParentView[1],
        median = evaResultsGraphData.ParentView[2],
        q3 = evaResultsGraphData.ParentView[3],
        high = evaResultsGraphData.ParentView[4],
        Color = "#FFB81C"

    };
    return new Series<BloxPlotSeriesData>
    {
        data = new List<BloxPlotSeriesData> { captiveViewSeriesData, parentViewSeriesData }
    };
}

private Series<BloxPlotSeriesData> GenerateBoxPlotChartSeries(NPVGraphData npvResultsGraphData)
{
    //CaptiveIns
    var captiveInsSeriesData = new BloxPlotSeriesData
    {
        low = npvResultsGraphData.CaptiveIns[0],
        q1 = npvResultsGraphData.CaptiveIns[1],
        median = npvResultsGraphData.CaptiveIns[2],
        q3 = npvResultsGraphData.CaptiveIns[3],
        high = npvResultsGraphData.CaptiveIns[4],
        Color = "#C111A0"
    };

    //CommIns
    var commInsSeriesData = new BloxPlotSeriesData
    {
        low = npvResultsGraphData.CommIns[0],
        q1 = npvResultsGraphData.CommIns[1],
        median = npvResultsGraphData.CommIns[2],
        q3 = npvResultsGraphData.CommIns[3],
        high = npvResultsGraphData.CommIns[4],
        Color = "#FFB81C"
    };
    //SelfIns
    var selfInsSeriesData = new BloxPlotSeriesData
    {
        low = npvResultsGraphData.SelfIns[0],
        q1 = npvResultsGraphData.SelfIns[1],
        median = npvResultsGraphData.SelfIns[2],
        q3 = npvResultsGraphData.SelfIns[3],
        high = npvResultsGraphData.SelfIns[4],
        Color = "#5D63D3"
    };
    return new Series<BloxPlotSeriesData>
    {
        data = new List<BloxPlotSeriesData>{ captiveInsSeriesData, commInsSeriesData, selfInsSeriesData }
    };

}
\$\endgroup\$
  • \$\begingroup\$ Is there a reason why you are not deserializing the CaptiveView, ParentView, etc directly into objects by using mapping instead of new'ing up the BloxPlotSeriesData class with the indexing? The code will be much easier to maintain and would be much easier to read. \$\endgroup\$ – Svek Mar 8 '18 at 11:47
  • \$\begingroup\$ The alternative, if there is some reason why you MUST index, then, I can see right away that you keep repeating the same mapping of low = 0, q1 = 1, ... high = 4, it is not too trivial to not repeat that code over and over. \$\endgroup\$ – Svek Mar 8 '18 at 11:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.