Kobe – MS New Web 2.0 Resource Kit in ASP.NET MVC and My thoughts

[Edit 2: Kobe is now pulled off from the ASP.NET front page as well as from the news section, the msdn page also has a red notice – congrats MS for this step.]

[Edit: There are few people who mentioned that MS is not claiming it as a Best Practice, but I think they are wrong as the moment I am typing this the www.asp.net still has this tag

“The resources are intended to guide you with the planning, architecting, and implementing of Web 2.0 applications and services”

and I do have the objection when it is labeled as guide]

If you recently visited the home page of www.asp.net you will find that Microsoft has released a new Resource Kit for developing Web 2.0 Applications. The resource kit contains a sample reference application that is developed in ASP.NET MVC framework. Since both of these are my area of interest, I downloaded the codes and give a quick walkthrough. Prior getting to the main discussion, I just want to remind you that it is the second reference application (after Oxite) that is named under Microsoft, so you are suppose to get some quality architecture/design, codes etc etc. But unfortunately, it does not even qualify as an average application that is written by joe developer,  it is filled with serious flaws in both architectural design and coding and I would consider it as a worst practise of developing ASP.NET MVC application. Let me list the serious issues that I found:

Full of Magic Values in Controller

The controllers contains interesting magic values and left me wondering what's wrong with Enum. Consider the following Action of Home Controller:

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Discovery(string query, string type, string section, string subSection, string page)
{
    query = Server.HtmlEncode(query);

    //Paging Section
    if (subSection != string.Empty)
    {
        if (type != "widget")
            return new ContentResult { Content = "" };

        int pageNo = 1;
        Int32.TryParse(page, out pageNo);
        if (pageNo == 0)
            pageNo = 1;

        if (section == "members")
        {
            List<UserSummary> users = null;

            switch (subSection)
            {
                case "members-mostactive":
                    users = _userService.GetMostActiveUsers(query, 8, pageNo);
                    break;
                case "members-all":
                    users = _userService.GetAllUsers(query, 8, pageNo);
                    break;
                default:
                    users = _userService.GetAllUsers(query, 8, pageNo);
                    break;
            }
            return View("MembersList", users);
        }
        else if (section == "groups")
        {
            List<Group> groups = null;

            switch (subSection)
            {
                case "groups-mostactive":
                    groups = _groupService.GetMostActiveGroupByKeyword(query, 8, pageNo);
                    break;
                case "groups-all":
                    groups = _groupService.GetAllGroupByKeyword(query, 8, pageNo);
                    break;
                default:
                    groups = _groupService.GetAllGroupByKeyword(query, 8, pageNo);
                    break;
            }
            return View("GroupsList", groups);
        }
        else if (section == "presentations")
        {

            List<PresentationSummary> presentations = null;
            switch (subSection)
            {
                case "presentations-all":
                    presentations = _presentationService.GetAllPresentationsByKewordTimeLine(query, "Day", "0", 10, pageNo);
                    break;
                case "presentations-mostpopular":
                    presentations = _presentationService.GetMostPopularPresentationsByKewordTimeLine(query, "Day", "0", 10, pageNo);
                    break;
                case "presentations-mostviewed":
                    presentations = _presentationService.GetMostViewedPresentationsByKewordTimeLine(query, "Day", "0", 10, pageNo);
                    break;
                case "presentations-mostdownload":
                    presentations = _presentationService.GetMostDownloadedPresentationsByKewordTimeLine(query, "Day", "0", 10, pageNo);
                    break;
                case "presentations-new":
                    presentations = _presentationService.GetNewPresentations(query, "Day", "0", 10, pageNo);
                    break;
                default:
                    presentations = _presentationService.GetAllPresentationsByKewordTimeLine(query, "Day", "0", 10, pageNo);
                    break;
            }
            return View("PresentationsList", presentations);
        }

        //    return new ContentResult { Content = "" };
    }

    return View();

}

Can you count the number of Magic string and integers in the above method?

Lack of Knowledge of ActionResult

Whenever the Action methods requires to return an empty result (which I think is also a design flaw) instead of EmptyResult it is returning new ContentResult { Content = "" }. Also when downloading a file, instead of using those nice FileResult, FilePathResult etc the people behind this application decided to modify the Response object directly (Check the Download method of Presentation Controller)  which clearly indicates the lack of knowledge of ASP.NET MVC Framework.

Action Methods are Populating Too Many View Data

Rather than populating the common view data with action filters or dividing the parts by Controller.RenderAction, the action methods are resposible to populate too many view data, consider the following action method:

        [AcceptVerbs(HttpVerbs.Get)]
        public ActionResult Index()
        {
            UserSummary memberOfDay = _communityService.GetUserOfDay();
            PresentationSummary presentationOfDay = _presentationService.GetPresentationOfDay();
            Group groupOfDay = _communityService.GetGroupOfDay();

            List<PresentationSummary> favouritePresentation = _communityService.GetCommunityFavorite(10,1); 
            List<PresentationSummary> userPresentationCount = _userService.GetAuthoredPresentations(memberOfDay.UserName);
            
            List<UserSummary> newUsers = _communityService.GetNewUsers(8,1);
            List<UserSummary> mostActiveUsers = _communityService.GetMostActiveUsers(8, 1);
            List<UserSummary> allUsers = _communityService.GetAllUsers(8, 1);
            int iNewMemberPageCount = decimal.Divide(_communityService.GetAllUsers().Count,8).ToInt();

            List<Group> newGroups = _communityService.GetNewGroups(8, 1);
            List<Group> mostActiveGroups = _communityService.GetMostActiveGroups(8, 1);
            List<Group> allGroups = _communityService.GetAllGroups(8, 1);
            int iNewGroupPageCount = decimal.Divide(_communityService.GetAllGroups().Count, 8).ToInt();
            int ifavouritesPageCount = 3;

            List<Group> groupsCommunity = _communityService.GetNewGroups(8,1);

            ViewData.Add("favouritePresentations", favouritePresentation.ToArray());
            ViewData.Add("UserOfDay", memberOfDay);
            ViewData.Add("GroupOfDay", groupOfDay);
            ViewData.Add("UserPresentationCount", userPresentationCount.Count);
            ViewData.Add("presentationsOfDay", presentationOfDay);            
            ViewData.Add("IsCommunityPage", true);

            ViewData.Add("members-new", newUsers);
            ViewData.Add("members-mostactive", mostActiveUsers);
            ViewData.Add("members-all", allUsers);
            ViewData.Add("members-totalcount", iNewMemberPageCount);

            ViewData.Add("groups-new", newGroups);
            ViewData.Add("groups-mostactive", mostActiveGroups);
            ViewData.Add("groups-all", allGroups);
            ViewData.Add("groups-totalcount", iNewGroupPageCount);

            ViewData.Add("favourites-totalcount", ifavouritesPageCount);


            ViewData.Add("GroupsListCommunity", groupsCommunity);


            return View();
        }

Same Action Method Returns Multiple Irrelevant View

Consider the following action:

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Profile(string userId, string type, string section, string subSection, string page)
{
    string argUserName = userId;

    UserSummary _user = null;
    _user = _userService.GetUserByName(argUserName);

    if (argUserName == HttpContext.User.Identity.Name)
        ViewData["UserLogged"] = "Yes";
    else
        ViewData["UserLogged"] = "No";

    ViewData["User"] = _user;
    //Paging Section
    if (subSection != string.Empty)
    {
        if (type != "widget")
            return new ContentResult { Content = "" };

        int pageNo = 1;
        Int32.TryParse(page, out pageNo);
        if (pageNo == 0)
            pageNo = 1;

        switch (section)
        {
            case "profileContacts":
                switch (subSection)
                {
                    case "profile-contacts":
                        AddContactsToViewData(_user, pageNo);
                        return View("UserContactList");
                    case "profile-groups":
                        AddGroupsToViewData(_user, pageNo);
                        return View("UserGroupList");
                    case "profile-authors":
                        AddAuthorsToViewData(_user, pageNo);
                        return View("UserAuthorList");
                }
                break;
            case "messages":
                switch (subSection)
                {
                    case "wall-messages":
                        AddUserMessagesToViewData(_user, pageNo);
                        return View("Messages", ViewData["wallMessage"]);
                    case "wall-userinvites":
                        AddUserInvitationToViewData(_user, pageNo);
                        return View("UserInvites", ViewData["ContactInvites"]);
                    case "wall-groupinvites":
                        AddGroupInviteToViewData(_user, pageNo);
                        return View("GroupInvites", ViewData["GroupInvites"]);
                }
                break;
            case "profile":
                switch (subSection)
                {
                    case "profile-recommendedForYou":
                        AddSystemRecommendationToViewData(_user, pageNo);
                        return View("SystemRecommended", ViewData["profile-recommendedForYou"]);
                    case "profile-contactRecommendations":
                        AddContactRecommendationToViewData(_user, pageNo);
                        return View("ContactsRecommended", ViewData["profile-contactRecommendations"]);
                    case "profile-myFavorites":
                        AddFavoriteToViewData(_user, pageNo);
                        return View("FavoritePresentations", ViewData["profile-myFavorites"]);
                    case "profile-myPresentations":
                        AddPresentationAuthoredToViewData(_user, pageNo);
                        return View("AuthoredPresentations", ViewData["profile-myPresentations"]);
                }
                break;

        }
    }

    return null;
}

The same action is responsible for returning 11 different view!!!

VB6 Style variable naming

I thought we have already passed those days, but it proves me wrong:

string regStatus = _userService.RegisterUser(username, passwordRegistration, firstname, lastname, email, arrInterests);

if (regStatus == "Success")
{
    UserSummary user = _userService.GetUserByName(username);
    string callBackURL = Request.Url.ToString().Replace("Registration", "EmailConfirmation") + "/" + user.UserId.ToString();
    string strMessageBody = GetRegistrationConfirmationMailBody(firstname, callBackURL);
    string strMailSubject = "PlanetPPT - confirm you email";
    this.SendMail("admin@planetppt.com", user.Email, strMessageBody, strMailSubject);

	//More Codes
}

Check that the UserService is returning string to indicate success/failure and the controller itself is responsible for formatting and sending the email.

Still Provider Model

The Data Access is designed with the similar style of Oxite v1.0, which the community has rejected a long time ago.

Zero Unit Test

The application does not contain a single line of unit test and the worst thing is that the way it has been architect there is zero chance that you will be able to write it without refactoring the architecture.

This is so far I think it worth to investigate and I can assure you that you will be able to find more flaws if you have the time and patience.

At last, my urge to Microsoft, please take down this application from both www.asp.net and MSDN or put a notice that the application is not up to the standard and do not follow it anyway as it is full of bad practices. Do introduce a review board and get the approval before releasing this kind of application. Remember this is not the first time that this kind of controversy was born, so please ensure the quality and standard.

Shout it

13 Comments

  • Great post. Can you please clarify a bit on provider model and what IS community accepted design patter for data access in ASP.NET MVC applications.

  • Hey Kazi, thanks for the feedback. I've sent an email to the Kobe team with your blog post and some of my feedback.

    I was part of a high-level review and I felt most of the app that I saw was reasonably put together.

    I think some of your "serious architectural issues" are really nits and matters of style. For example, is the VB6 naming issue really a "serious architectural issue"?

    I might call it a serious "style" issue for sure. ;) And I do think they should correct it.

    I agree with you about the Profile action. It seems like it could be cleaned up with some smarter routing into multiple action methods.

    The EmptyResult issue is probably just a minor oversight, easily corrected.

    Do you feel the majority of the action methods and applications are problematic? Or are the cases relatively minor?

  • In response to this post, I wanted to refactor until this codebase was something that the community might think is good, but I read the Kobe EULA and AFAICT, it is not allowed.

    In fact, my reading of the EULA is that the code snippets in this blog post are a violation of the EULA and of the original copyright holder.

    So not only is it bad guidance, but we can't share that guidance with anyone else nor can we fix the badness of it.

  • @lilconnorpeterson: Try Oxite v2.0, it is MS application and has some good things in it.

  • Ok, well they'll start to address these things promptly. They didn't intend to be a best-practices document, but they also realize it's important to not reinforce any bad practices in the first place.

    Thanks for bringing up the feedback!

  • @Jay: Sorry I do have very little knowledge on those legal issues, so until I get any objection I will keep the codes that should help the readers to understand the flaws.

  • @Haccked: Thanks for your quick response.

  • These codesamples make my head hurt. Shouldn't controller be trim ?

  • Thanks for the Oxite v2 mention. Appreciate it. :)

  • @Erik Porter: Sure man, I always try to appreciate what is good.

  • Kazi,

    Thank you for your feedback on Kobe. I appreciate your interest to explore Kobe implementation. We’ll incorporate your feedback and update it soon. Stay tuned!

  • "Thursday, April 16, 2009 4:30 PM by kazimanzurrashid

    @lilconnorpeterson: Try Oxite v2.0, it is MS application and has some good things in it."

    Sorry, my google-fu fails me. What's Oxite v2.0?

    Is this something different from the latest version posted on codeplex (Oxite.2009.2.15)?

    Thank you in advance!

  • @Masashi : Great.

Comments have been disabled for this content.