public HttpResponseMessage Post(LongUrlModel model)
{
if (ModelState.IsValid)
{
{
ModelState.AddModelError("", "You cant shorten a URL that belongs to this domain");
}

*snip*
}
}


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.

• what does snip mean? And also, is this a MVC WebApi project? – dreza Jun 29 '14 at 1:53

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)
{
}

{
ModelState.AddModelError("", "You cant shorten a URL that belongs to this domain");
}

// Perform the valid code logic here
}

{
}


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.

1. 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

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

1. 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
}
}

• Thanks bro. You couldn't have known this, but I am using Web API 2 so I used the standard BadRequest method. I also reduced the nesting like you suggested. A custom validation attribute is a nice idea, as is a validator class, but as the validation is only required in one place (the aforementioned action) I do not feel as though the ROI makes it worthwhile. – Caster Troy Jun 29 '14 at 16:47
• @CasterTroy ah ok, fair enough. The only other suggestion then is perhaps moving that validation to a private method within the controller i.e. if(IsShortUrlRequest()) return BadRequest() – dreza Jun 29 '14 at 20:15