Nie testuję kontrolerów - część 1

Nie testuję kontrolerów - część 1

Przykłady z sieci

Ostatnio w sieci ukazało się wiele przykładów jak posprzątać kod w kontrolerach mvc. Sam podjąłem próbę walki z ciągle powtarzającym się kodem. Rozdmuchany kontroler z akcjami wypchanymi kodem wszelakiej maści, od walidacji po wywołania serwisów, repozytoriów, łapanie wyjątków i jak do tego jeszcze zaplącze się jakaś cząstka logiki biznesowej nie przyniesie nam nic dobrego. Prosty przykład:

[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);
}

Wygląda fatalnie prawda? A jeszcze jak kontroler ma kilka akcji to już kompletna kaszanka. Tu kolejny przykładzik:

public ViewResult Index(int id)
{
    GroupViewModel group = Mapper.Map<Group, GroupViewModel>(groupService.GetGroup(id));
    group.Goals = Mapper.Map<IEnumerable<GroupGoal>, IEnumerable<GroupGoalViewModel>>(groupGoalService.GetGroupGoals(id));

    foreach (var item in group.Goals)
    {
        var user = userService.GetUser(item.GroupUser.UserId);

        item.UserId = user.Id;
        item.User = user;
    }
    var assignedgroupuser = groupUserService.GetGroupUser(User.Identity.GetUserId(), id);
    if (assignedgroupuser != null)
    {
        group.GoalsAssignedToOthers = Mapper.Map<IEnumerable<GroupGoal>, IEnumerable<GroupGoalViewModel>>(groupGoalService.GetAssignedGoalsToOthers(assignedgroupuser.GroupUserId));
        group.GoalsAssignedToMe = Mapper.Map<IEnumerable<GroupGoal>, IEnumerable<GroupGoalViewModel>>(groupGoalService.GetAssignedGoalsToMe(assignedgroupuser.GroupUserId));
    }
    group.Focus = focusService.GetFocussOFGroup(id);

    //group.GroupUserId = groupUserService.GetGroupUserByuserId(((SocialGoalUser)(User.Identity)).UserId).GroupUserId;
    group.Users = groupUserService.GetMembersOfGroup(id);
    //if (group.GroupUser.UserId == ((SocialGoalUser)(User.Identity)).UserId)
    if (groupUserService.GetAdminId(id) == User.Identity.GetUserId())
        group.Admin = true;
    var status = 0;
    foreach (var item in group.Users)
    {
        if (item.Id == (User.Identity.GetUserId()))
            status = 1;
    }
    if (status == 1)
        group.IsAMember = true;
    if (groupRequestService.RequestSent((User.Identity.GetUserId()), id))
        group.RequestSent = true;
    if (groupInvitationService.IsUserInvited(id, (User.Identity.GetUserId())))
        group.InvitationSent = true;
    return View("Index", group);
}

Brrrr... Na sam widok człowiek może dostać drgawki. Niestety w wielu przykładach na sieci (powyższe wycinki też znalezione na sieci) taki kod to normalka. Gołym okiem widać, że kontroler, którego zadaniem jest koordynowanie zadań między modelem a widokiem robi dużo za dużo. Przydało by się posprzątać. :)

Dobra. Miałem tego nie wstawiać, ale wrzucam jeszcze konstruktor:

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;
}

Uch... Spociłem się... Wow... Niezłe co? :)

Rozwiązanie

Ok. Starczy tego. Co byście powiedzieli, gdyby konstruktor wyglądał tak:

public GroupController() { }

Tak. Dobrze myślicie. Jak widać jawna deklaracja konstruktora w ogóle nie jest potrzebna. Zatem akcja odpowiedzialna za wyświetlenie formatki tworzenia nowej grupy:

[Get]
public ActionResult Create()
{
    return View<CreateView>();
}

A wyświetlenie formatki do edycji tak:

[Get]
public ActionResult Update(int id)
{
    return View<Group, UpdateView>(id);
}

No dobra. To jeszcze akcja Update typu POST:

[Post]
public ActionResult Update(UpdateRequest model)
{
    return Handle(model)
        .With(rq => Dispatcher.Execute(rq))
        .OnSuccess(rq => this.RedirectTo(x => x.Update(rq.ID)))
        .OnSuccessNotify(Resources.UpdateSuccessNotify)
        .OnFailure(rq => this.RedirectTo(x => x.Update(rq.ID)));
}

Wygląd tych akcji wyciągnąłem z kodu projektu nad którym obecnie pracuję. To efekt szukania i zebrania w całość wszelkich pomysłów własnych jak i tych znalezionych na sieci.

Co zatem kryje się pod metodami View<> i Handle? O tym w kolejnym poście.

PS

Acha... Chyba teraz już powoli zaczyna być widoczne dlaczego nie testuję kontrolerów. Jeśli jednak to za mało, to mam nadzieję, że przekonam Was w kolejnych wpisach. :)

Komentarze