patterncsharpMinor
WebApi action that updates entity using stored procedure
Viewed 0 times
storedwebapiactionprocedurethatusingupdatesentity
Problem
I have ASP.NET WebApi project. Some of controller has method
SetAsMain(int id) that get information about people then use stored procedure update people. How can I improve it? Should I do check people.IsMain == true in stored procedure?public HttpResponseMessage SetAsMain(int id)
{
People people = repository.GetById(id);
if (people == null)
{
return ErrorMsg(HttpStatusCode.NotFound, string.Format("No people with ID = {1}", id));
}
if (people.IsMain == true)
{
return ErrorMsg(HttpStatusCode.OK, string.Format("{0} is already set as main", people.Houses.ToString()));
}
try
{
var parameters = new Dictionary();
parameters.Add("PeopleID", id.ToString());
repository.ExecProcedure("usp_PeopleSetMain", parameters);
return Request.CreateResponse(HttpStatusCode.OK, repository.GetById(id)); // some problem here
}
catch (Exception ex)
{
return ErrorMsg(HttpStatusCode.InternalServerError, ex.Message);
}
}Solution
- You don't have a
people, you have aperson.
- Saying
if (person.IsMain == true)is exactly equivalent toif (person.IsMain)
Updated code below.
public HttpResponseMessage SetAsMain(int id)
{
People person = repository.GetById(id);
if (people == null)
{
return ErrorMsg(HttpStatusCode.NotFound, string.Format("No people with ID = {1}", id));
}
if (person.IsMain)
{
return ErrorMsg(HttpStatusCode.OK, string.Format("{0} is already set as main", person.Houses.ToString()));
}
try
{
var parameters = new Dictionary();
parameters.Add("PeopleID", id.ToString());
repository.ExecProcedure("usp_PeopleSetMain", parameters);
return Request.CreateResponse(HttpStatusCode.OK, repository.GetById(id));
}
catch (Exception ex)
{
return ErrorMsg(HttpStatusCode.InternalServerError, ex.Message);
}
}I would also recommend changing the name of the repository method that returns a person from
GetById to GetPersonById. As it is currently, the method name gives us no clue what's going on. I had to figure it out from context.Code Snippets
public HttpResponseMessage SetAsMain(int id)
{
People person = repository.GetById(id);
if (people == null)
{
return ErrorMsg(HttpStatusCode.NotFound, string.Format("No people with ID = {1}", id));
}
if (person.IsMain)
{
return ErrorMsg(HttpStatusCode.OK, string.Format("{0} is already set as main", person.Houses.ToString()));
}
try
{
var parameters = new Dictionary<string, string>();
parameters.Add("PeopleID", id.ToString());
repository.ExecProcedure("usp_PeopleSetMain", parameters);
return Request.CreateResponse(HttpStatusCode.OK, repository.GetById(id));
}
catch (Exception ex)
{
return ErrorMsg(HttpStatusCode.InternalServerError, ex.Message);
}
}Context
StackExchange Code Review Q#104676, answer score: 3
Revisions (0)
No revisions yet.