Additional model validation

public HttpResponseMessage Post(LongUrlModel model)
{
    if (ModelState.IsValid)
    {
        var baseAddress = Request.RequestUri.ToString().TrimEnd('/');
        if (model.LongUrl.StartsWith(baseAddress))
        {
            ModelState.AddModelError("", "You cant shorten a URL that belongs to this domain");
            return Request.CreateErrorResponse(HttpStatusCode.BadRequest, ModelState);
        }

        *snip*
    }
    return Request.CreateErrorResponse(HttpStatusCode.BadRequest, ModelState);
}

I am not at all fond of the conditional logic here, it can be quite hard to follow. How can I refactor the additional model validation logic so that it will co-exist with the standard model validation nicely?

I feel as though there could be a way to refactor this code so that Request.CreateErrorResponse only occurs in the code once, but I cannot see how at the moment.

In regards to your question:


I feel as though there could be a way to refactor this code so that
Request.CreateErrorResponse only occurs in the code once, but I cannot
see how at the moment.

I would consider reducing the nesting (Arrow code) of the If statements and perform follow the return early mantra.

public HttpResponseMessage Post(LongUrlModel model)
{
    if (!ModelState.IsValid)
    {
        return BadRequest();
    }

    var baseAddress = Request.RequestUri.ToString().TrimEnd('/');
    if (model.LongUrl.StartsWith(baseAddress))
    {
        ModelState.AddModelError("", "You cant shorten a URL that belongs to this domain");
        return BadRequest();
    }

    // Perform the valid code logic here
}

private HttpResponseMessage BadRequest()
{
    return Request.CreateErrorResponse(HttpStatusCode.BadRequest, ModelState);
}

In regards to your other question


I am not at all fond of the conditional logic here

There's a few things I can think of you could try.

-
Create a custom validation attribute and decorate the LongUrl property.

public class UrlLengthAttribute: ValidationAttribute
  {
      public UrlLengthAttribute()
      {
      }

      protected override ValidationResult IsValid(object value, ValidationContext validationContext)
      {
        var httpContext = new HttpContextWrapper(HttpContext.Current);
        var url = value as string;

        if(string.isNullOrWhiteSpace())
        {
          return base.IsValid(value, validationContext);
        }

        // I'm not 100% on this code but you get the general idea
        var baseAddress = httpContext.RequestUri.ToString().TrimEnd('/');

        if(url.StartsWith(baseAddress))
        {
          return new ValidationResult(this.FormatErrorMessage(validationContext.DisplayName));
        }

        return null;
      }
  }

You can then use this to decorate your model:

public class LongUrlModel
    {
        [UrlLength(ErrorMessage = "You cant shorten a URL that belongs to this   domain")]    
        public string LongUrl { get; set; }
    }

-
You could the extract the model validation out into a separate Validator class and use that in the controller instead. At the very least it might be moving towards the lines of creating a unit testable module.

public class LongUrlModelValidator
  {
     public LongUrlModelValidator(HttpContextBase httpContext)
     {
      // store and use
     }

     public bool Validate(LongUrlModel model)
     {
      // perform validation logic here
     }
  }

StackExchange Code Review Q#55573, answer score: 3