Wrapper class for the Rotten Tomatoes API

Question

I'm working on a site that has to do with movies. I am consuming the Rotten Tomatoes API to get my movie information and using EF5 with a code first approach to get that into the database. I'm using MVC4. I am struggling to come up with a design that makes sense and accomplishes what I need to in an efficient manner while still following the Single Responsibility Principle.

A quick summary:

Users can have a collection of movies, so every movie that is returned to the client needs to have information like whether the requesting user has that movie in their collection, the user rating, etc. When a user adds a movie to their collection in the database I also insert that movie (into the database). So I'm only inserting movies that users own, as opposed to every movie returned from Rotten Tomatoes.

I think it makes sense to walk you through the action of a user searching for a movie to best show you my problems/questions...

A user searches for a movie and makes a request to my controller. I've created a wrapper class for the Rotten Tomatoes API, so the controller instantiates my RottenTomatoes object then calls the FindMovies() method.

Controller

[HttpGet]
public JsonResult GetMovieSearchResults(string name) {
    if(!string.IsNullOrEmpty(name)) {
        var tomatoes = new RottenTomatoes(ConfigurationManager.AppSettings["RottenTomatoesApiKey"]);
        return Json(tomatoes.FindMovies(name), JsonRequestBehavior.AllowGet);
    }

    return Json(new { success = false }, JsonRequestBehavior.AllowGet);
}

The FindMovies() method calls GetResponse() which makes a call to the API endpoint, deserializes the returned json into MoviesDataContract and then returns that DataContract back to FindMovies().

RottenTomatoes wrapper class

public class RottenTomatoes {
    private const string MOVIES_SEARCH_URL = "http://api.rottentomatoes.com/api/public/v1.0/movies.json?apikey={0}&q={1}&page_limit={2}";
    private Guid _userId;
    private MovieContext _db;

    public string ApiKey { get; set; }

    public RottenTomatoes(string apiKey, Guid userId, MovieContext movieContext) {
        ApiKey = apiKey;
        _userId = userId;
        _db = movieContext;
    }

    public IEnumerable<Movie> FindMovies(string name, int pageLimit = 20) {
        string url = string.Format(MOVIES_SEARCH_URL, ApiKey, name, pageLimit);
        var movies = GetResponse(url).Movies.Select(m => (Movie)m);

        // populate the UserMovies property for each movie
        var userMovies = _db.UserMovies.Where(u => u.UserId == _userId);
        foreach(var movie in movies) {
            var userMovie = userMovies.FirstOrDefault(u => u.Movie.Id == movie.Id);
            if(userMovie != null) {
                movie.UserMovies.Add(new UserMovie {
                    IsWatched = userMovie.IsWatched,
                    Rating = userMovie.Rating
                }); 
            }
        }

        return movies;
    }

    private MoviesDataContract GetResponse(string url) {
        var request = (HttpWebRequest)WebRequest.Create(url);
        request.Method = WebRequestMethods.Http.Get;
        request.ContentType = "application/json; charset=utf-8";

        try {
            using (var response = (HttpWebResponse)request.GetResponse()) {
                var serializer = new DataContractJsonSerializer(typeof(MoviesDataContract));
                return (MoviesDataContract)serializer.ReadObject(response.GetResponseStream());
            }
        } catch (Exception e) {
            throw new Exception(e.Message);
        }
    }
}

I think up to this point, the design is smooth. I am struggling with it going forward.

FindMovies() receives back MoviesDataContract from GetResponse() and has the responsibilty of converting the IEnumerable<> of movies in MoviesDataContract to an IEnumerable<> of my EF model Movie. See questions 1 & 2

I created an explicit conversion operator in my EF model Movie class that will except a movie from my DataContract and convert it to the EF model. Here's the EF Model Movie class.

public class Movie {
    [Key]
    [DatabaseGenerated(DatabaseGeneratedOption.None)]
    public int Id { get; set; }

    [MaxLength(100)]
    public string Title { get; set; }

    public int? Year { get; set; }

    public virtual ICollection<UserMovie> UserMovies { get; set; } 

    public Movie() {
        UserMovies = new HashSet<UserMovie>();
    }

    public static explicit operator Movie(MoviesDataContract.Movie rawMovie) {
        var movie = new Movie {
            Id = rawMovie.Id,
            Title = rawMovie.Title,
            Year = rawMovie.Year
        };

        return movie;
    }
}

public class UserMovie {
    [Key]
    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public int Id { get; set; }

    public int? Rating { get; set; }

    public bool IsWatched { get; set; }

    public Guid UserId { get; set; }

    [ScriptIgnore]
    public virtual Movie Movie { get; set; }
}

Once the movie has been converted to my EF Model Movie I now need to give each movie information about the current user. This is where I struggle the most. Should the RottenTomatoes class have the responsibility of populating this information, should the controller, or should I create a new wrapper class around RottenTomatoes, or perhaps a better solution all together? See question 3

If RottenTomatoes has the responsibility, then I need to worry about passing in the DataContext to get user information about each movie, and the user id of the auhtenticated user. Fine, but it seems like too many dependencies and it seems thatRottenTomatoes shouldn't need to know anything about a DataContext or user id.

Once user information is set the result set is returned back to the controller where it is serialized and sent back to the client.

Questions

1. Should I convert all movies from my DataContract to my EF model right away as soon as they're returned from RottenTomates, like I'm doing currently, or only when a user is adding a movie to their collection? Is there a best practice for this sort of thing?
2. Should FindMovies() have the responsibility of converting the DataContract into Movie, my EF model? Really, should the RottenTomatoes class have that responsibility at all?
3. Should RottenTomatoes be responsible for adding user information about each movie before it returns the result set back to the controller? If not, is there a better design or perhaps a design pattern that could help me here?

Answer 1

This looks really silly to me:

    try {
        using (var response = (HttpWebResponse)request.GetResponse()) {
            var serializer = new DataContractJsonSerializer(typeof(MoviesDataContract));
            return (MoviesDataContract)serializer.ReadObject(response.GetResponseStream());
        }
    } catch (Exception e) {
        throw new Exception(e.Message);
    }

You create an exception that throws an exception. It just doesn't make sense to me. If you want the exception to bubble up, then you should just get rid of the try catch altogether.

It would be different if you returned a "bad response" so that the calling object knew that it was bad for some reason, so that it could tell the user that something went wrong.

You already have a Using Statement in there so that if there is an issue the response is still disposed, so you should just let the exception bubble up naturally.

using (var response = (HttpWebResponse)request.GetResponse()) {
    var serializer = new DataContractJsonSerializer(typeof(MoviesDataContract));
    return (MoviesDataContract)serializer.ReadObject(response.GetResponseStream());
}