tonykaralis / TmdbEasy

A light-weight wrapper for the TMDb Api written in C#. TmdbEasy makes getting movie data a piece of cake..
MIT License
6 stars 4 forks source link

SearchByActorAsync wrong return type #5

Open PoLaKoSz opened 6 years ago

PoLaKoSz commented 6 years ago

In TMDb v3 the search/person endpoint response different from the search/movies so the response can not be Task<MovieList>.

tonykaralis commented 6 years ago

That makes sense, I did this so a while back and can't remember what it returns.

tonykaralis commented 6 years ago

This is a simple fix.

tonykaralis commented 6 years ago

Having looked into this in more detail, this is what it returns

public class RootObject
    {
        public int page { get; set; }
        public List<Result> results { get; set; }
        public int total_results { get; set; }
        public int total_pages { get; set; }
    }
 public class Result
    {
        public string profile_path { get; set; }
        public bool adult { get; set; }
        public int id { get; set; }
        public List<KnownFor> known_for { get; set; }
        public string name { get; set; }
        public double popularity { get; set; }
    }
public class KnownFor
{
    public string poster_path { get; set; }
    public bool adult { get; set; }
    public string overview { get; set; }
    public string release_date { get; set; }
    public string original_title { get; set; }
    public List<object> genre_ids { get; set; }
    public int id { get; set; }
    public string media_type { get; set; }
    public string original_language { get; set; }
    public string title { get; set; }
    public string backdrop_path { get; set; }
    public double popularity { get; set; }
    public int vote_count { get; set; }
    public bool video { get; set; }
    public double vote_average { get; set; }
    public string first_air_date { get; set; }
    public List<string> origin_country { get; set; }
    public string name { get; set; }
    public string original_name { get; set; }
}

Passing in an actors name will return all actors that match eg. brad pi may return multiple results. Brad pitt on the other hand will return a single result. Point is if the user types in the correct name then thats ok, but what if they dont??

tonykaralis commented 6 years ago
  1. We can either write logic to filter out the actual actor which will be difficult if they have typed it in very incorrectly. or
  2. we can build up a list of actors from their search and allow them to pick the correct actor. Then we can perform the search and be gauranteed to get only 1 result. We can then extract all the movies from the known for object and add use those as needed.

I will implement option 2 and see how that works.

PoLaKoSz commented 6 years ago

I think it would be easier to loop through on the found actors and extract every Movie from the results.

tonykaralis commented 6 years ago

Definitely easier. Are you happy with returning all movies for all actors found(even incorrect ones)?

PoLaKoSz commented 6 years ago

My idea not fully good because I thought like we would use it in MoodMovies. This library should return the deserialized response and the MoodMovies responsibility to extract only the movies from every actors / actresses.

tonykaralis commented 6 years ago

Yeah 100% agree. Moodmovies can implement point 2 that I mentioned above.

PoLaKoSz commented 5 years ago

Hi!

Lucky for Us there is an another problem :smile:

The response results.(actor object).known_for list can contains two types of object:

My ideas:

Do You have any other idea? Maybe the names represented in the code example not the best, Can You give your thoughts about it? Thanks

tonykaralis commented 5 years ago

Hi,

I agree with your first point that sounds dirty.

Your second point makes more sense but will that mean that one of those lists will be null when a query is run and returned to the user?

Sorry I am a bit detached from this as havent worked with it for a while, did you find this out from the docs on TMDB?

PoLaKoSz commented 5 years ago

No, my religion deny using null-s 😄 I against using null-s no matter what. Ctor for initializing everything.

In 2009 Tony Hoare (C.A.R. Hoare) stated that he invented the null reference in 1965 as part of the ALGOL W language. In that 2009 reference Hoare describes his invention as a "billion-dollar mistake":

I call it my billion-dollar mistake. It was the invention of the null reference in 1965. At that time, I was designing the first comprehensive type system for references in an object oriented language (ALGOL W). My goal was to ensure that all use of references should be absolutely safe, with checking performed automatically by the compiler. But I couldn't resist the temptation to put in a null reference, simply because it was so easy to implement. This has led to innumerable errors, vulnerabilities, and system crashes, which have probably caused a billion dollars of pain and damage in the last forty years.

Source: Wikipedia / Null pointer / History


With the second implementation the MixedSearchResults class would be the JSON known_for array. So the two List<T> should be populated (I think this needs a custom Deserializer) and returned from the SearchByActorAsync( ... ).

tonykaralis commented 5 years ago

Hahahah good one 👍 i think we can add a custom deserializer as long as we dont touch the already existing Json code. We can another method that can deserialize this tricky type. Are you up for the challenge?

PoLaKoSz commented 5 years ago

What do You mean by the already existing Json code?

Yes, You can assign this to me (but maybe it will take a few days to implement and commit it).

tonykaralis commented 5 years ago

Have you had any luck with this this?

PoLaKoSz commented 5 years ago

Hi!

I will implement it today.

Can I implement breaking changes?

tonykaralis commented 4 years ago

@PoLaKoSz once v1 is merged then I think we can look at fixing this. It could be done using a custom json serialization plugin perhaps but something to look at after v1