1
\$\begingroup\$

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);
    }
\$\endgroup\$
1
\$\begingroup\$

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
    };
}
\$\endgroup\$
4
  • \$\begingroup\$ Thanks when you say constructor to FlyoutMenuLinkItem that accepts three arguments is that the private method ? \$\endgroup\$
    – Jefferson
    Nov 16 '21 at 13:06
  • \$\begingroup\$ @Jefferson the constructor is for the FlyoutMenuLinkItem, if you cannot change FlyoutMenuLinkItem constructors, then you create a private method with the same arguments just like the last example in my answer. \$\endgroup\$
    – iSR5
    Nov 16 '21 at 13:10
  • \$\begingroup\$ So in the model I would add a constructor ? \$\endgroup\$
    – Jefferson
    Nov 16 '21 at 13:12
  • 1
    \$\begingroup\$ @Jefferson yes, that's correct. \$\endgroup\$
    – iSR5
    Nov 16 '21 at 13:13

Your Answer

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

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