HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Self-contained poll application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
containedpollapplicationself

Problem

I am working on a small and self-contained poll application, not unlike Straw Poll whereby one user can submit a poll and other users can vote (but only once).

I am building this little application in the open, using Angular for the front-end and Web Api for the back-end. I am using Fluent Validation to validate incoming models.

Using an action filter and Fluent Validation I have managed to remove all basic model validation from my controller.

I need to carry out some more advanced validation after I load the poll from the database. My action method has become overrun by such validation logic as you can see:

public IHttpActionResult Put(int pollId, VoteInput voteInput)
{
    var poll = _session.Load(pollId);
    var voterIp = Request.GetOwinContext().Request.RemoteIpAddress;

    if (poll == null)
    {
        return BadRequest("you cannot vote on a poll that does not exist.");
    }

    if (poll.VoterIps.Contains(voterIp))
    {
        return BadRequest("you cannot vote on this poll because you have already voted.");
    }

    if (voteInput.Options.Length > 1 && !poll.MultiChoice)
    {
        return BadRequest("you cannot vote for more than one option. noob.");
    }

    foreach (var option in poll.Options.Where(option => voteInput.Options.Contains(option.Id)))
    {
        option.Votes += 1;
    }

    poll.VoterIps.Add(voterIp);
    _session.SaveChanges();

    return Ok();
}


I would very much like to remove some, if not all of this validation logic from the controller.

I have a really sound infrastructure - I am using Fluent Validation and a DI Framework - but owing to my inexperience with said technologies, I cannot think of a clean way to extract the validation logic.

What are some ways I can reduce or eliminate the validation in my controller?

Solution

I cannot think of a clean way to extract the validation logic.

All validation logic should be in an appropriate "validation service" class as @meWantToLearn said.

I assume something like this is possible in FluentValidation:

public class PollVoteValidator : AbstractValidator


If not then make a simple class that contains both of these and then create a validator for that:

public class PollInput {
    public Poll {get; set;}
    public VoteInput {get; set;}
}

public class PollVoteValidator : AbstractValidator


Validation Separation of Concerns

Validations such as if (poll == null) belong in a PollValidator class and if (poll.VoterIps.Contains(voterIp)) in a VoterIpsCollection class.

Even if you want to check for null method arguments up front (and that will simplify downstream control logic), the above still applies.

Validation Coordination

The PollVoteValidator would coordinate everything perhaps, with logic, somewhere, like this:

public class PollVoteValidator {
    public ValidateResults Validate (poll, voteInput) {
        var pollValidateResults = myPollValidator.Validate(poll);
        var voteInputValidateResults = myVoteInputValidator.Validate(voteInput);
        // I hope poll is not null!
        var voterIpsCollectionResults = myVoterIpsCollectionValidator.Validate(poll.VoterIps);

        if(/* validations above pass */)
            pollVoteValidatorResults = mypollVoteValidator.Validate(poll, voteInput);

        // consolidate validation results as desired (error messages)
        // for return to the caller

        return pollVoteValidatorResults;
    }
}


One More Thing

This: var voterIp = Request.GetOwinContext().Request.RemoteIpAddress; seems "orphaned" from an OO perspective. voterIp feels like a Voter class property, so maybe you should instantiate a Voter; this implies a "validation service" class for Voter.

The above Voter class stuff may seem like overkill, but there is no OO principle that says "if something seems simple, ignore OO principles".

Code Snippets

public class PollVoteValidator : AbstractValidator<Poll,VoteInput>
public class PollInput {
    public Poll {get; set;}
    public VoteInput {get; set;}
}

public class PollVoteValidator : AbstractValidator<PollInput>
public class PollVoteValidator {
    public ValidateResults Validate (poll, voteInput) {
        var pollValidateResults = myPollValidator.Validate(poll);
        var voteInputValidateResults = myVoteInputValidator.Validate(voteInput);
        // I hope poll is not null!
        var voterIpsCollectionResults = myVoterIpsCollectionValidator.Validate(poll.VoterIps);

        if(/* validations above pass */)
            pollVoteValidatorResults = mypollVoteValidator.Validate(poll, voteInput);

        // consolidate validation results as desired (error messages)
        // for return to the caller

        return pollVoteValidatorResults;
    }
}

Context

StackExchange Code Review Q#75787, answer score: 4

Revisions (0)

No revisions yet.