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

WebAPI controller

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

Problem

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

///
/// Get information related with a job by providing "Custome surname" and "VRN".
///
/// Customer surname.
/// Vehicle registration number.
/// Returns a collection of fields related to the job.
[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.Crea

Solution

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.

Code Snippets

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

Context

StackExchange Code Review Q#67452, answer score: 4

Revisions (0)

No revisions yet.