Web-service for wiki-like movie database

Question

I'm making a web-service for an app with a (partially) crowdsourced movie database.

These two functions allows users to add and remove which genres a movie is liked to, and get the log of whom have done what changes.

I need to make many very similar functions for other data.

    [HttpPost]
    [Authorize(Roles = "User")]
    public IHttpActionResult PostMovieGenre([FromBody]MovieGenreBindingModel edit)
    {
        string[] IDs = edit.genre_ids.Split(',');

        using (var db = new DBEntities())
        {
            var movie = db.movies.Find(edit.movie_id);

            if (movie == null)//Confirm that the movie was found
                return BadRequest("Movie not found");

            foreach (string sid in IDs)
            {
                int g_id = Convert.ToInt32(sid);
                if (edit.action == "add")
                {
                    movie.movies_genres.Add(new movies_genres
                    {
                        movies_id = edit.movie_id,
                        genres_id = g_id
                    });
                }
                else if (edit.action == "remove")
                {
                    var m_g = movie.movies_genres.Where(g => g.genres_id == g_id).First(); //Find the movies_genres object
                    db.movies_genres.Remove(m_g); //Remove it
                }
                else
                {
                    return BadRequest("Invalid action");
                }

                db.movies_genres_changes.Add(new movies_genres_changes //Log the change
                {
                    action = edit.action,
                    genres_id = g_id,
                    movies_id = edit.movie_id,
                    time = DateTime.UtcNow,
                    user_id = UserManager.FindByName(User.Identity.Name).Id
                });
            }
            try
            {
                db.SaveChanges();
            }
            catch (Exception e)
            {
                return BadRequest();
            }
        }
        return Ok();
    }


    [ResponseType(typeof(List<RMovieGenresChange>))]
    [HttpGet]
    [Authorize(Roles = "User")]
    public IHttpActionResult GetMovieGenresChanges(int movie_id)
    {
        List<RMovieGenresChange> retchanges = new List<RMovieGenresChange>();

        using (var db = new DBEntities())
        {
            try
            {
                var dbRevs = db.movies.Find(movie_id).movies_genres_changes;

                if (dbRevs == null)
                    return BadRequest("No revision for an movie entry with id " + movie_id);

                foreach (movies_genres_changes m in dbRevs)
                {
                    retchanges.Insert(0, new RMovieGenresChange() // Put newest change first in list
                    {
                        action = m.action,
                        genre = new RGenre
                        {
                            id = m.genres_id,
                            name = m.genres.name
                        },
                        movie_id = m.movies_id,
                        time = m.time,
                        user = new RUserShort
                        {
                            user_id = m.user_id,
                            user_name = User.Identity.Name
                        }
                    });
                }
            }
            catch (Exception e)
            {
                return BadRequest();
            }
        }

        return Ok(retchanges);
    }

Answer 1

A general comment: Extract logic to small private methods. Note that your methods are long which makes it hard to understand what they do. Every independent piece of code should be extracted into a private method. Like

private void RemoveMovie(id)

If you use the Add method on a List object it will insert the first item into the first place the second to the second and so on. No need in retchanges.Insert(0, new RMovieGenresChange()

GetMovieGenresChanges is a good method since it does only one thing unlike PostMovieGenre method which handles both the addition and the removal of the geners. This is a violation of the Single Responsibility Principle and hence you should split it to two methods: one for the addition AddGenereToMovie and another for the removal - RemoveGenereFromMovie. This way yo keep your methods short, clean, readable and maintainable.