WebAPI controller

Question

It return an object with WebAPI by proving some data. Is this a good approach?

using MWM.Database.Model;
using MWM.Entity.API;
using MWM.Repository;
using MWM.Services;
using System.Net;
using System.Net.Http;
using System.Web.Http;
namespace MWM.Web.Controllers
{
 public class JobController : ApiController
 {
 private readonly IJobRepository _jobRepo;
 private readonly JobFacade _jobFacade;
 public JobController()
 {
 // Init repository
 MwmContext context = new MwmContext();
 // WebAPI is not able to serialize objects with this feature enabled.
 context.Configuration.ProxyCreationEnabled = false;
 _jobRepo = new JobRepository(context);
 _jobFacade = new JobFacade(_jobRepo);
 }
 /// <summary>
 /// Get information related with a job by providing "Custome surname" and "VRN".
 /// </summary>
 /// <param name="surname">Customer surname.</param>
 /// <param name="vrn">Vehicle registration number.</param>
 /// <returns>Returns a collection of fields related to the job.</returns>
 [Authorize(Users = "WEBSERVICE")]
 public HttpResponseMessage Get(string surname, string vrn)
 {
 try
 {
 if (string.IsNullOrEmpty(surname) || string.IsNullOrEmpty(vrn))
 {
 var message = string.Format("You need to provide both surname and VRN, in order to get some job infomation. (Surname='{0}', VRN='{1}')", surname, vrn);
 return Request.CreateErrorResponse(HttpStatusCode.NotFound, message);
 }
 APIJob job = _jobFacade.GetJobBySurnameVRN(surname, vrn);
 if (job == null)
 {
 var message = string.Format("Sorry, but there is no job with surname = {0} and VRN = {1} not finished was found.", surname, vrn);
 return Request.CreateErrorResponse(HttpStatusCode.NotFound, message);
 }
 return Request.CreateResponse(HttpStatusCode.OK, job);
 }
 catch (System.Exception)
 {
 return Request.CreateErrorResponse(HttpStatusCode.NotFound, "");
 }
 }
 }
}

UPDATE:

This is how the method looks now after the code review:

[HttpGet]
[Authorize(Users = "webservice")]
public IHttpActionResult Get(string surname, string vrn)
{
 try
 {
 if (string.IsNullOrEmpty(surname) || string.IsNullOrEmpty(vrn))
 return BadRequest("Please, provide customer surname and VRN");
 ApiJob job = _jobFacade.GetApiJob(surname, vrn);
 if (job == null) return NotFound();
 return Ok(job);
 }
 catch (System.Exception)
 {
 return Conflict();
 }
}

Answer 1

I see this pattern from time to time but I don't like it: GetJobBySurnameVRN or GetJobById. That's what overloads and perhaps named arguments are for: all you care about is the GetJob part, everything else is just one specific way of retrieving the data but does not have to actually be a separate method rather than an overload.

I would change it to GetJob and call it like this (and considering your variables are aptly named it can stay like this): GetJob(surname, vrn). If for some reason you don't like this, you can always do GetJob(surname: surname, vrn: vrn) but that doesn't add anything so use it when the variable actually isn't clear (like GetJob(surname: param[0], vrn: param[1])).

---

Have you considered Web Api 2? It changes this

return Request.CreateErrorResponse(HttpStatusCode.NotFound, "lala");

into

return NotFound("lala");

You'll have these helper methods available for several status codes.

---

Furthermore, not entirely sure if it's limited to Web Api or Web Api 2 but I prefer to explicitly define the action's attributes: [HttpGet]. Likewise Web Api 2 provides very handy (and very, very configurable) [Route] attributes.