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.