Jak nie robić MVC

zawodowo Komentarze

Jak nie robić MVC

Jakiś czas temu od znajomego dostałem link do przykładowej aplikacji, która wg. autora ma pokazać wzorce dla:

Pomyślałem, że będzie fajna lekturka, fajnych przykładów nigdy za wiele. Niestety bardzo szybko okazało się, że prezentowany kod daleko odbiega od założeń. A jak otworzyłem kod jednego z kontrolerów moim oczom ukazał się las…

public GroupController(
    IGroupService groupService,
    IGroupUserService groupUserService,
    IUserService userService,
    IMetricService metricService,
    IFocusService focusService,
    IGroupGoalService groupgoalService,
    IGroupInvitationService groupInvitationService,
    ISecurityTokenService securityTokenService,
    IGroupUpdateService groupUpdateService,
    IGroupCommentService groupCommentService,
    IGoalStatusService goalStatusService,
    IGroupRequestService groupRequestService,
    IFollowUserService followUserService,
    IGroupCommentUserService groupCommentUserService,
    IGroupUpdateSupportService groupUpdateSupportService,
    IGroupUpdateUserService groupUpdateUserService)
{
    this.groupService = groupService;
    this.groupInvitationService = groupInvitationService;
    this.userService = userService;
    this.groupUserService = groupUserService;
    this.metricService = metricService;
    this.focusService = focusService;
    this.groupGoalService = groupgoalService; ;
    this.securityTokenService = securityTokenService;
    this.groupUpdateService = groupUpdateService;
    this.groupCommentService = groupCommentService;
    this.goalStatusService = goalStatusService;
    this.groupRequestService = groupRequestService;
    this.followUserService = followUserService;
    this.groupCommentUserService = groupCommentUserService;
    this.groupUpdateSupportService = groupUpdateSupportService;
    this.groupUpdateUserService = groupUpdateUserService;
}

...las serwisów. Jak to zobaczyłem to aż złapałem się za głowę. Dalej nie jest lepiej. Przykładowa metoda typu POST z tego samego kontrolera:

    [HttpPost]
    public ActionResult CreateGroup(GroupFormModel newGroup)
    {
        var userId = User.Identity.GetUserId();
        Group group = Mapper.Map<GroupFormModel, Group>(newGroup);
        var errors = groupService.CanAddGroup(group).ToList();
        ModelState.AddModelErrors(errors);
        if (ModelState.IsValid)
        {
            var createdGroup = groupService.CreateGroup(group, userId);
            return RedirectToAction("Index", new { id = createdGroup.GroupId });
        }
        return View("CreateGroup", newGroup);
    }

Później można trafić jeszcze na takie kwiatki jak:

Powyższe przykłady obrazują kilka problemów. Pierwszy z nich to wielgachny konstruktor. Samo to daje mocno do myślenia, że jest tu coś nie tak. Kolejny to powtarzalność sporej ilości kodu w kontrolerze. W pokazanej wyżej akcji CreateGroup można zauważyć standardowy kod, który przewija się przez większość akcji typu POST, czyli:

Pomijam zupełnie fakt, że użycie AutoMapper'a do bezmyślnego skopiowania danych z modelu widoku do modelu domeny jest anty-wzorcem DDD, ale tego tematu nie będę poruszał, przynajmniej na razie.

Z akcjami typu GET jest ponownie. Tak samo wyglądające kawałki kodu, które robią prawie to samo, a kod kontrolera staje się rozbudowany. Na koniec okazuje się, że kontroler z kilkoma akcjami zajmuje nam kilkaset linii, zamiast kilkadziesiąt.

To nie jest recenzja aplikacji SocialGoal. Przypadek spowodował, że akurat ten kod wpadł mi w łapki właśnie wtedy kiedy zbierałem materiały do tego wpisu. Czego jeszcze należy unikać tworząc aplikacje mvc?

Komentarze