JsonResult Building menu

Question

This logic checks different method and adds links to response that is returned to a view. I there a better way to do this ?

    [HttpGet]
    [StandaloneResponseFilter]
    public JsonResult GetMenuLinkList()
    {
        List<FlyoutMenuLinkItem> response = new List<FlyoutMenuLinkItem>();

            try
            {
                var IsDisplayLink = ProfileHelpers.DisplayLink();
                var IsGroupLink = ProfileHelpers.DisplayGroupLink();
                var favoritePages = ProfileHelpers.GetFavoritePages();

                if (IsDisplayLink){
                    FlyoutMenuLinkItem fmlMI = new FlyoutMenuLinkItem();
                    fmlMI.PageName = "Portal";
                    fmlMI.PageURL = "/members/portal";
                    fmlMI.PageSection = FlyoutMenuLinkItem.Section.MemberPortal;
                    response.Add(fmlMI);
                }

                if (IsGroupLink )
                {
                    FlyoutMenuLinkItem fmlMD = new FlyoutMenuLinkItem();
                    fmlMD.PageName = "Group Link";
                    fmlMD.PageURL = "/meetings/disclosures";
                    fmlMD.PageSection = FlyoutMenuLinkItem.Section.MeetingsDisclosures;
                    response.Add(fmlMD);
                }

                if (favoritePages != null)
                {
                    foreach (FavoritePage f in favoritePages)
                    {
                        FlyoutMenuLinkItem fmlFav = new FlyoutMenuLinkItem();
                        fmlFav.PageName = f.PageName;
                        fmlFav.PageURL = f.Url;
                        fmlFav.PageSection = FlyoutMenuLinkItem.Section.MyPages;

                        response.Add(fmlFav);
                    }
                }


            }
            catch (Exception e)
            {
                Log.Write(e, ConfigurationPolicy.ErrorLog);
            }

        }

        return Json(response, JsonRequestBehavior.AllowGet);
    }

Answer 1

it seems DisplayLink() , DisplayGroupLink() have a bad naming, which made you store them in a variable with a better naming. if you're working on old code that has not followed good practices, I would suggest you avoid using it directly, and instead, implement a new class that would follow good practices, and use it instead. Then, you can then mark old code as an Obsolete.

For your code portion, you can either create a method to create a menu link OR (better) add a constructor to FlyoutMenuLinkItem that accepts three arguments you need to create a new FlyoutMenuLinkItem :

public class FlyoutMenuLinkItem
{
    FlyoutMenuLinkItem() { }
    FlyoutMenuLinkItem(string name, string url, FlyoutMenuLinkItem.Section section) { ... }
}

Now, you can do this :

if (IsDisplayLink)
{
    response.Add(new FlyoutMenuLinkItem("Portal", "/members/portal", FlyoutMenuLinkItem.Section.MemberPortal);
}


if (IsGroupLink)
{
    response.Add(new FlyoutMenuLinkItem("Group Link", "/meetings/disclosures", FlyoutMenuLinkItem.Section.MeetingsDisclosures);
}


if (favoritePages != null)
{
    response.AddRange(favoritePages.Select(x=> new FlyoutMenuLinkItem(x.PageName, x.Url, FlyoutMenuLinkItem.Section.MyPages)));
}

Also, a private method would do the same if you don't have access to FlyoutMenuLinkItem :

private FlyoutMenuLinkItem CreateMenuLink(string name, string url, FlyoutMenuLinkItem.Section section)
{
    return new FlyoutMenuLinkItem
    {
        PageName = name,
        PageURL = url,
        PageSection = section
    };
}