Return json to browser via MVC controller for morris chart

Question

Program is written in .Net Framework 4.

Class:

public class yearsComparison
{
    public int year { get; set; }
    public string material1 { get; set; }
    public string material2 { get; set; }
    public string material3 { get; set; }
    public string material4 { get; set; }
    public string material5 { get; set; }
    public decimal value1 { get; set; }
    public decimal value2 { get; set; }
    public decimal value3 { get; set; }
    public decimal value4 { get; set; }
    public decimal value5 { get; set; }
    public string vendor1 { get; set; }
    public string vendor2 { get; set; }
    public string vendor3 { get; set; }
    public string vendor4 { get; set; }
    public string vendor5 { get; set; }
}

Code to change data into json, it will return top five over purchase material from current year and last year:

public ActionResult OverPurchaseToYear()
{
    var overPurchaseToYear = DAL.ReportHandler.GetOverPurchaseYearToYear(DateTime.Now.Year, 5);
    List<yearsComparison> chartData = new List<yearsComparison>();
    int index = 1;
    foreach (var item in overPurchaseToYear)
    {
        if (chartData.Where(x => x.year == item.forecastYear).Any())
        {
            yearsComparison temp = chartData.Where(x => x.year == item.forecastYear).FirstOrDefault();
            switch (index)
            {
                case 1:
                    temp.material1 = item.material;
                    temp.value1 = (decimal)item.overPurchaseQuantity;
                    temp.vendor1 = item.emVendor;
                    break;
                case 2:
                    temp.material2 = item.material;
                    temp.value2 = (decimal)item.overPurchaseQuantity;
                    temp.vendor2 = item.emVendor;
                    break;
                case 3:
                    temp.material3 = item.material;
                    temp.value3 = (decimal)item.overPurchaseQuantity;
                    temp.vendor3 = item.emVendor;
                    break;
                case 4:
                    temp.material4 = item.material;
                    temp.value4 = (decimal)item.overPurchaseQuantity;
                    temp.vendor4 = item.emVendor;
                    break;
                case 5:
                    temp.material5 = item.material;
                    temp.value5 = (decimal)item.overPurchaseQuantity;
                    temp.vendor5 = item.emVendor;
                    break;
            }
        }
        else
        {
            yearsComparison temp = new yearsComparison();
            temp.year = item.forecastYear;
            switch (index)
            {
                case 1:
                    temp.material1 = item.material;
                    temp.value1 = (decimal)item.overPurchaseQuantity;
                    temp.vendor1 = item.emVendor;
                    break;
                case 2:
                    temp.material2 = item.material;
                    temp.value2 = (decimal)item.overPurchaseQuantity;
                    temp.vendor2 = item.emVendor;
                    break;
                case 3:
                    temp.material3 = item.material;
                    temp.value3 = (decimal)item.overPurchaseQuantity;
                    temp.vendor3 = item.emVendor;
                    break;
                case 4:
                    temp.material4 = item.material;
                    temp.value4 = (decimal)item.overPurchaseQuantity;
                    temp.vendor4 = item.emVendor;
                    break;
                case 5:
                    temp.material5 = item.material;
                    temp.value5 = (decimal)item.overPurchaseQuantity;
                    temp.vendor5 = item.emVendor;
                    break;
            }
            chartData.Add(temp);
        }
        index++;
    }
    var json = chartData.ToArray();
    var jsonData = Json(json, JsonRequestBehavior.AllowGet);
    return jsonData;
}

The json has to be in this format:

[
{"year":2018,"material1":"00-00012","material2":"00-00011","material3":"00-00010","material4":"00-00009","material5":"00-00008","value1":220.00,"value2":210.00,"value3":200.00,"value4":190.00,"value5":180.00,"vendor1":"12345","vendor2":"12345","vendor3":"12345","vendor4":"12345","vendor5":"12345"},
{"year":2017,"material1":"00-00012","material2":"00-00011","material3":"00-00010","material4":"00-00009","material5":"00-00008","value1":220.00,"value2":210.00,"value3":200.00,"value4":190.00,"value5":180.00,"vendor1":"12345","vendor2":"12345","vendor3":"12345","vendor4":"12345","vendor5":"12345"}
]

The generated chart will look like this

Code works but ugly, how can I improve it?

Answer 1

The switch statement is repeated code that can be refactored into a single purpose function

private void populate(int index, MyItemType item, yearsComparison model) {
    switch (index) {
        case 1:
            model.material1 = item.material;
            model.value1 = (decimal)item.overPurchaseQuantity;
            model.vendor1 = item.emVendor;
            break;
        case 2:
            model.material2 = item.material;
            model.value2 = (decimal)item.overPurchaseQuantity;
            model.vendor2 = item.emVendor;
            break;
        case 3:
            model.material3 = item.material;
            model.value3 = (decimal)item.overPurchaseQuantity;
            model.vendor3 = item.emVendor;
            break;
        case 4:
            model.material4 = item.material;
            model.value4 = (decimal)item.overPurchaseQuantity;
            model.vendor4 = item.emVendor;
            break;
        case 5:
            model.material5 = item.material;
            model.value5 = (decimal)item.overPurchaseQuantity;
            model.vendor5 = item.emVendor;
            break;
    }
}

These 3 lines

var json = chartData.ToArray();
var jsonData = Json(json, JsonRequestBehavior.AllowGet);
return jsonData;

can be reduced to one line

return Json(chartData, JsonRequestBehavior.AllowGet);

The former serves no real purpose as List<T> and Array serialize to the same JSON type.

The Where filter is also repeated unnecessarily on chartData when what you are actually checking for is if the item already exists in the collection.

With the suggested changes the action refactors to

public ActionResult OverPurchaseToYear() {
    var overPurchaseToYear = DAL.ReportHandler.GetOverPurchaseYearToYear(DateTime.Now.Year, 5);
    var chartData = new List<yearsComparison>();
    int index = 1;
    foreach (var item in overPurchaseToYear) {
        yearsComparison row = chartData.Where(x => x.year == item.forecastYear).FirstOrDefault();
        if (row != null) {
            populate(index, item, row);
        } else {
            row = new yearsComparison();
            row.year = item.forecastYear;
            populate(index, item, row);
            chartData.Add(row);
        }
        index++;
    }
    return Json(chartData, JsonRequestBehavior.AllowGet);
}

Answer 2

Most things are covered by @NKosi, so I won't repeat them. However you should consider this suggestion once you are done with changes suggested by him. So ideally this is not a complete answer but the continuation to what @NKosi said.

Your C# model violates the naming guidelines and it will not pass static code analysis. I understand the library you mentioned wont work with capitalized keys in JSON output. So what you can do is use DataMember or JsonProperty attribute on your model, use a serializer that support these properties and return result from custom JSON ActionResult. This way you will adhere to naming standard in C# as well as JSON.

See how you can write custom ActionResult for this: https://stackoverflow.com/a/12497902/1440057