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

MVC-Web API 2 integration

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

Problem

For the first time ever, I'm integrating an MVC website to an ASP.NET Web API 2 web service (both of which are coded by myself).

I'd appreciate it if you can just look this over for me and let me know if I'm on the right track here as far as the integration goes.

I'm working based on this tutorial but I had a little trouble along the way.

What I need to know is:

  • Am I missing anything in either the service controller or the client controller that I should have - any oversight?



  • Is my MVC controller totally thread safe (i.e. is anything going to be blocked with the asyncronysity?



  • Is there a better or more efficient way to do anything that I'm doing here?



This is just a simple request/respond scenario where the web service acts as the middle-man between the application and the database so it's really nothing special.

Here's the code for the web service controller:

```
public class UserController : ApiController
{
OrtundServiceEntities db = new OrtundServiceEntities();

public HttpResponseMessage Get()
{
List users = db.Users.ToList();

HttpResponseMessage response = Request.CreateResponse(HttpStatusCode.OK, users);

return response;
}

public HttpResponseMessage Get(int Id)
{
User u = db.Users.Single(x => x.Id == Id);

HttpResponseMessage response;
if (u != null)
response = Request.CreateResponse(HttpStatusCode.OK, u);
else
response = Request.CreateResponse(HttpStatusCode.NotFound);

return response;
}

[Route("~/api/user/login")]
public HttpResponseMessage Post([FromBody]User Source)
{
User requestedUser = db.Users.Single(x => x.EmailAddress == Source.EmailAddress);

HttpResponseMessage response;

if (requestedUser != null && Hashing.ValidatePassword(Source.Password, requestedUser.Password))
response = Request.CreateResponse(HttpStatusCode.OK, requestedUser);
else
response

Solution

Avoid duplicated magic strings, like "http://www.ortund.com/". Assign to a constant instead.

This snippet appears repeated multiple times:

client.BaseAddress = new Uri("http://www.ortund.com/");
client.DefaultRequestHeaders.Accept.Clear();
client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));


It would be better to move common logic to a helper method.

It's recommended to catch the most specific type of exception,
instead of catch (Exception ex).
Otherwise you risk masking a truly unexpected failure.

The name of the ThrowError method suggests some sort of helper that throws an exception. But that's not the case: it returns an HTTP error response.

It's not a good idea to name classes after your own name.
You should name classes after their functionality,
typically the abstract data type they represent.

Instead of storing the value to return in a local variable, like this:

HttpResponseMessage response;
if (u != null)
    response = Request.CreateResponse(HttpStatusCode.OK, u);
else
    response = Request.CreateResponse(HttpStatusCode.NotFound);

return response;


It's somewhat more readable and safer to return immediately, for example:

if (u != null)
{
    return Request.CreateResponse(HttpStatusCode.OK, u);
}
return Request.CreateResponse(HttpStatusCode.NotFound);

Code Snippets

client.BaseAddress = new Uri("http://www.ortund.com/");
client.DefaultRequestHeaders.Accept.Clear();
client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
HttpResponseMessage response;
if (u != null)
    response = Request.CreateResponse(HttpStatusCode.OK, u);
else
    response = Request.CreateResponse(HttpStatusCode.NotFound);

return response;
if (u != null)
{
    return Request.CreateResponse(HttpStatusCode.OK, u);
}
return Request.CreateResponse(HttpStatusCode.NotFound);

Context

StackExchange Code Review Q#95608, answer score: 3

Revisions (0)

No revisions yet.