WebApi action that updates entity using stored procedure

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<string, string>();

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

• I was afraid of this when I saw your previous question: why are you using a stored proc to update this one property? Also, this seems contradictory to me: ErrorMsg(HttpStatusCode.OK -- what is it, an error or not an error? – BCdotWEB Sep 15 '15 at 9:01
• This stored proc not only update one property its do many thing. About ErrorMsg: I have to create a similar method with other name. Thanks – Roman Marusyk Sep 15 '15 at 9:29

• You don't have a people, you have a person.
• Saying if (person.IsMain == true) is exactly equivalent to if (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<string, string>();

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.

• Thanks. A second time I want to get not the same instance of people, it should return a updated data after run stored proc. but it does not work. Could you please suggest me? – Roman Marusyk Sep 15 '15 at 9:52
• Ahh. Yes. Good catch. You are getting a new instance back after it's been updated. You can ignore that part of my answer. I was wrong about that. – RubberDuck Sep 15 '15 at 10:39
• Yes, but it return me the same instance as I had before without changes :(. What is I doing wrong? Thx. – Roman Marusyk Sep 15 '15 at 11:01
• @MegaTron I think it makes sense to do the check where you do it. If it's already set to be "main", there's no sense in making another round trip to the database. – RubberDuck Sep 15 '15 at 11:39
• Your welcome. Good luck with working out your other issue. One thing to check for is if your sproc is actually updating the record. – RubberDuck Sep 15 '15 at 11:41