4
\$\begingroup\$

Here's a method inside my controller that reads the values of angle and point data from my database. Then it grabs the data and adds it to a new list and sends the JSON to the view.

I can't simplify this simply putting my if statements into two ActionResults because I can only bind one datasource to one kendo grid.

[OutputCache(NoStore = true, Duration = 0, VaryByParam = "*")]
        public ActionResult ReadMeasurements([DataSourceRequest] DataSourceRequest request, string viewType)
        {
            JsonResult json = new JsonResult();
            List<AngleData> angledata = UserSession.GetValue(StateNameEnum.Planning, ScreenName.Planning.ToString() + "Angles" + viewType, UserSessionMode.Database) as List<AngleData>;
            List<PointData> pointData = UserSession.GetValue(StateNameEnum.Planning, ScreenName.Planning.ToString() + "Points" + viewType, UserSessionMode.Database) as List<PointData>;
            if(pointData != null && angledata != null) 
            {

                List<PlanningViewParam> angles = new List<PlanningViewParam>();
                foreach (AngleData i in angledata)
                {
                    string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(i.color.r * 255), (int)(i.color.g * 255), (int)(i.color.b * 255))).Name.Remove(0, 2);
                    int angleVal = (int)i.angleValue;
                    angles.Add(new PlanningViewParam()
                    {
                        Color = col,
                        Label = "Angle",
                        Value = angleVal,
                        Number = i.angleNumber
                    });
                }
                List<DPlanningViewParam> points = new List<PlanningViewParam>();
                foreach (PointData f in pointData)
                {
                    string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(f.color.r * 255), (int)(f.color.g * 255), (int)(f.color.b * 255))).Name.Remove(0, 2);
                    string pointAnglesVal = f.pointAnglesValue;
                    points.Add(new PlanningViewParam()
                    {
                        Color = col,
                        Label = "Point",
                        ValueTwo = pointAnglesVal,
                        Number = f.pointNumber
                    });
                }
                return Json(new { Angles =  angles, Points = points }, JsonRequestBehavior.AllowGet);
            }
            if (angledata != null)
            {
                List<PlanningViewParam> angles = new List<PlanningViewParam>();
                foreach (AngleData i in angledata)
                {
                    string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(i.color.r * 255), (int)(i.color.g * 255), (int)(i.color.b * 255))).Name.Remove(0, 2);
                    int angleVal = (int)i.angleValue;
                    angles.Add(new PlanningViewParam()
                    {
                        Color = col,
                        Label = "Angle",
                        Value = angleVal,
                        Number = i.angleNumber
                    });
                }
                return json = Json(angles.ToDataSourceResult(request, i => new PlanningViewParam()
                {
                    Color = i.Color,
                    Label = i.Label,
                    Value = i.Value,
                    Number = i.Number
                }), JsonRequestBehavior.AllowGet);

            }
            if (pointData != null)
            {
                List<PlanningViewParam> points = new List<PlanningViewParam>();
                foreach (PointData f in pointData)
                {
                    string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(f.color.r * 255), (int)(f.color.g * 255), (int)(f.color.b * 255))).Name.Remove(0, 2);
                    string pointAnglesVal = f.pointAnglesValue;
                    points.Add(new PlanningViewParam()
                    {
                        Color = col,
                        Label = "Point",
                        ValueTwo = pointAnglesVal,
                        Number = f.pointNumber
                    });
                }
                return json = Json(points.ToDataSourceResult(request, f => new PlanningViewParam()
                {
                    Color = f.Color,
                    Label = f.Label,
                    Value = f.Value,
                    Number = f.Number
                }), JsonRequestBehavior.AllowGet);

            }

            return null;

        }
\$\endgroup\$
0

2 Answers 2

Reset to default
7
\$\begingroup\$

First of all, get rid of this line:

JsonResult json = new JsonResult();

Just use return new JSON(...).

Now, you have repeating chunks of codes when you construct the angles and points Lists. I recommend you extract them to separate methods. If you do not want to clutter your code with the methods use the delegate() or Func<>() to create a functions inside your method.

So, with that your code will be simpler:

    public ActionResult ReadMeasurements([DataSourceRequest] DataSourceRequest request, string viewType)
    {
        List<AngleData> angledata = UserSession.GetValue(StateNameEnum.Planning, ScreenName.Planning.ToString() + "Angles" + viewType, UserSessionMode.Database) as List<AngleData>;
        List<PointData> pointData = UserSession.GetValue(StateNameEnum.Planning, ScreenName.Planning.ToString() + "Points" + viewType, UserSessionMode.Database) as List<PointData>;
        if(pointData != null && angledata != null) 
        {

            List<PlanningViewParam> angles = BuildAngles(angledata);
            List<DPlanningViewParam> points = BuildPoints(pointData);
            return new Json(new { Angles =  angles, Points = points }, JsonRequestBehavior.AllowGet);
        }
        else if (angledata != null)
        {
            List<PlanningViewParam> angles = BuildAngles(angledata);
            return new Json(angles.ToDataSourceResult(request, i => new PlanningViewParam()
            {
                Color = i.Color,
                Label = i.Label,
                Value = i.Value,
                Number = i.Number
            }), JsonRequestBehavior.AllowGet);

        }
        else if (pointData != null)
        {
            List<DPlanningViewParam> points = BuildPoints(pointData);
            return new Json(points.ToDataSourceResult(request, f => new PlanningViewParam()
            {
                Color = f.Color,
                Label = f.Label,
                Value = f.Value,
                Number = f.Number
            }), JsonRequestBehavior.AllowGet);

        }

        return null;

    }

BTW, I noticed that the JSON for the first condition pointData != null && angledata != null is returned differently. You return just new { Angles = angles, Points = points } allowing the .Net engine to serialise the data for you. For other conditions you explicitly list all elements. You either did not test the first condition, or the explicitly listing all elements is not required, as the engine does the job for just fine. If the latter is the case then use return new Json(new { Angles = angles }, JsonRequestBehavior.AllowGet); instead of angles.ToDataSourceResult(request, i... Try this and see how it goes:

    public ActionResult ReadMeasurements([DataSourceRequest] DataSourceRequest request, string viewType)
    {
        List<AngleData> angledata = UserSession.GetValue(StateNameEnum.Planning, ScreenName.Planning.ToString() + "Angles" + viewType, UserSessionMode.Database) as List<AngleData>;
        List<PointData> pointData = UserSession.GetValue(StateNameEnum.Planning, ScreenName.Planning.ToString() + "Points" + viewType, UserSessionMode.Database) as List<PointData>;
        if(pointData != null && angledata != null) 
        {

            List<PlanningViewParam> angles = BuildAngles(angledata);
            List<DPlanningViewParam> points = BuildPoints(pointData);
            return new Json(new { Angles =  angles, Points = points }, JsonRequestBehavior.AllowGet);
        }
        else if (angledata != null)
        {
            List<PlanningViewParam> angles = BuildAngles(angledata);
            return new Json(new { Angles =  angles }, JsonRequestBehavior.AllowGet);

        }
        else if (pointData != null)
        {
            List<DPlanningViewParam> points = BuildPoints(pointData);
            return new Json(new { Points = points }, JsonRequestBehavior.AllowGet);

        }

        return null;

    }
\$\endgroup\$
3
  • \$\begingroup\$ Cool, thank you so much! I'll review it soon and mark as answer when I get it done. Thank you. \$\endgroup\$
    – Kala J
    Commented Apr 16, 2015 at 13:57
  • \$\begingroup\$ I didn't get the chance to refactor until now but I can't remove: JsonResult json = new JsonResult(); because it doesn't recognize json. @cha \$\endgroup\$
    – Kala J
    Commented May 19, 2015 at 15:19
  • \$\begingroup\$ Also for BuildAngles and BuildPoints, I would return lists of angle and point data? \$\endgroup\$
    – Kala J
    Commented May 19, 2015 at 15:27
3
\$\begingroup\$

You check if both are not null and then you check them both again, I see from the returns that you don't need to run another block of code when you end another block. personally I think I would set this up differently.

If one is null, check to see which one isn't null and run that code, otherwise neither are null and run the else statement

public ActionResult ReadMeasurements([DataSourceRequest] DataSourceRequest request, string viewType)
{
    JsonResult json = new JsonResult();
    List<AngleData> angledata = UserSession.GetValue(StateNameEnum.Planning, ScreenName.Planning.ToString() + "Angles" + viewType, UserSessionMode.Database) as List<AngleData>;
    List<PointData> pointData = UserSession.GetValue(StateNameEnum.Planning, ScreenName.Planning.ToString() + "Points" + viewType, UserSessionMode.Database) as List<PointData>;
    if(pointData == null || angledata == null) 
    {
        if (angledata != null)
        {
            List<PlanningViewParam> angles = new List<PlanningViewParam>();
            foreach (AngleData i in angledata)
            {
                string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(i.color.r * 255), (int)(i.color.g * 255), (int)(i.color.b * 255))).Name.Remove(0, 2);
                int angleVal = (int)i.angleValue;
                angles.Add(new PlanningViewParam()
                {
                    Color = col,
                    Label = "Angle",
                    Value = angleVal,
                    Number = i.angleNumber
                });
            }
            return json = Json(angles.ToDataSourceResult(request, i => new PlanningViewParam()
            {
                Color = i.Color,
                Label = i.Label,
                Value = i.Value,
                Number = i.Number
            }), JsonRequestBehavior.AllowGet);

        }
        else if (pointData != null)
        {
            List<PlanningViewParam> points = new List<PlanningViewParam>();
            foreach (PointData f in pointData)
            {
                string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(f.color.r * 255), (int)(f.color.g * 255), (int)(f.color.b * 255))).Name.Remove(0, 2);
                string pointAnglesVal = f.pointAnglesValue;
                points.Add(new PlanningViewParam()
                {
                    Color = col,
                    Label = "Point",
                    ValueTwo = pointAnglesVal,
                    Number = f.pointNumber
                });
            }
            return json = Json(points.ToDataSourceResult(request, f => new PlanningViewParam()
            {
                Color = f.Color,
                Label = f.Label,
                Value = f.Value,
                Number = f.Number
            }), JsonRequestBehavior.AllowGet);
        }           
    }
    else 
    {
        List<PlanningViewParam> angles = new List<PlanningViewParam>();
        foreach (AngleData i in angledata)
        {
            string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(i.color.r * 255), (int)(i.color.g * 255), (int)(i.color.b * 255))).Name.Remove(0, 2);
            int angleVal = (int)i.angleValue;
            angles.Add(new PlanningViewParam()
            {
                Color = col,
                Label = "Angle",
                Value = angleVal,
                Number = i.angleNumber
            });
        }
        List<DPlanningViewParam> points = new List<PlanningViewParam>();
        foreach (PointData f in pointData)
        {
            string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(f.color.r * 255), (int)(f.color.g * 255), (int)(f.color.b * 255))).Name.Remove(0, 2);
            string pointAnglesVal = f.pointAnglesValue;
            points.Add(new PlanningViewParam()
            {
                Color = col,
                Label = "Point",
                ValueTwo = pointAnglesVal,
                Number = f.pointNumber
            });
        }
        return Json(new { Angles =  angles, Points = points }, JsonRequestBehavior.AllowGet);
    }
    return null;
}

I noticed that you have some code duplication here, if you could normalize the pointData and the angleData a little bit more so that they were the same type you could actually reduce your code by about 67% by using a method like:

public List<PlanningViewParam> getPlanningViewParams(List<AngleData> data)
{
    List<PlanningViewParam> dataPoints = new List<PlanningViewParam>();
    foreach (AngleData f in data)
    {
        string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(f.color.r * 255), (int)(f.color.g * 255), (int)(f.color.b * 255))).Name.Remove(0, 2);
        string pointAnglesVal = f.pointAnglesValue;
        dataPoints.Add(new PlanningViewParam()
        {
            Color = col,
            Label = "Point",
            ValueTwo = pointAnglesVal,
            Number = f.pointNumber
        });
    }
    return dataPoints;
}

right now just getting rid of the duplication around the angleData you get this...

public ActionResult ReadMeasurements([DataSourceRequest] DataSourceRequest request, string viewType)
{
    JsonResult json = new JsonResult();
    List<AngleData> angleData = UserSession.GetValue(StateNameEnum.Planning, ScreenName.Planning.ToString() + "Angles" + viewType, UserSessionMode.Database) as List<AngleData>;
    List<PointData> pointData = UserSession.GetValue(StateNameEnum.Planning, ScreenName.Planning.ToString() + "Points" + viewType, UserSessionMode.Database) as List<PointData>;
    if(pointData == null || angledata == null) 
    {
        if (angledata != null)
        {
            List<PlanningViewParam> angles = getPlanningViewParams(angleData);
            return json = Json(angles.ToDataSourceResult(request, i => new PlanningViewParam()
            {
                Color = i.Color,
                Label = i.Label,
                Value = i.Value,
                Number = i.Number
            }), JsonRequestBehavior.AllowGet);

        }
        else if (pointData != null)
        {
            List<PlanningViewParam> points = new List<PlanningViewParam>();
            foreach (PointData f in pointData)
            {
                string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(f.color.r * 255), (int)(f.color.g * 255), (int)(f.color.b * 255))).Name.Remove(0, 2);
                string pointAnglesVal = f.pointAnglesValue;
                points.Add(new PlanningViewParam()
                {
                    Color = col,
                    Label = "Point",
                    ValueTwo = pointAnglesVal,
                    Number = f.pointNumber
                });
            }
            return json = Json(points.ToDataSourceResult(request, f => new PlanningViewParam()
            {
                Color = f.Color,
                Label = f.Label,
                Value = f.Value,
                Number = f.Number
            }), JsonRequestBehavior.AllowGet);
        }           
    }
    else 
    {
        List<PlanningViewParam> angles = getPlanningViewParams(angleData);
        List<DPlanningViewParam> points = new List<PlanningViewParam>();
        foreach (PointData f in pointData)
        {
            string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(f.color.r * 255), (int)(f.color.g * 255), (int)(f.color.b * 255))).Name.Remove(0, 2);
            string pointAnglesVal = f.pointAnglesValue;
            points.Add(new PlanningViewParam()
            {
                Color = col,
                Label = "Point",
                ValueTwo = pointAnglesVal,
                Number = f.pointNumber
            });
        }
        return Json(new { Angles =  angles, Points = points }, JsonRequestBehavior.AllowGet);
    }
    return null;
}

public List<PlanningViewParam> getPlanningViewParams(List<AngleData> data)
{
    List<PlanningViewParam> dataPoints = new List<PlanningViewParam>();
    foreach (AngleData f in data)
    {
        string col = "#" + ColorTranslator.FromHtml(String.Format("#{0:X2}{1:X2}{2:X2}", (int)(f.color.r * 255), (int)(f.color.g * 255), (int)(f.color.b * 255))).Name.Remove(0, 2);
        string pointAnglesVal = f.pointAnglesValue;
        dataPoints.Add(new PlanningViewParam()
        {
            Color = col,
            Label = "Point",
            ValueTwo = pointAnglesVal,
            Number = f.pointNumber
        });
    }
    return dataPoints;
}

from the looks of it, you could probably make AngleData and PointData the same data type and merge them into the same and then you could really clean up this code.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.