> * Actors could have as many as 5 actors (according to
> entertainerlib.medialibrary.videos.Movie) so you'll need to slice the
> actor_list to actor_list[:5] instead of actor_list[:3] or else we may lose
> actors. It looks like you were just indenting this line of code, but this is
> just something I happened to notice.
> * Just like above, but with directors and in the opposite direction. There
> will only ever be two directors according to the Movie class so the slice
> should probably just be to :2.
> * Same story with writers. There will only ever be two, according to the
> Movie class.
Yep that's right. I've committed those changes.
> * Did you think about making a common method since this seems to be a
> repeated pattern here? Maybe it's not worth it with because of the number of
> size and position variables, but I was just curious if you considered it.
This is something I constantly try to do when it reduces code drastically or makes it more readable.
Matt hi,
> * Actors could have as many as 5 actors (according to medialibrary. videos. Movie) so you'll need to slice the
> entertainerlib.
> actor_list to actor_list[:5] instead of actor_list[:3] or else we may lose
> actors. It looks like you were just indenting this line of code, but this is
> just something I happened to notice.
> * Just like above, but with directors and in the opposite direction. There
> will only ever be two directors according to the Movie class so the slice
> should probably just be to :2.
> * Same story with writers. There will only ever be two, according to the
> Movie class.
Yep that's right. I've committed those changes.
> * Did you think about making a common method since this seems to be a
> repeated pattern here? Maybe it's not worth it with because of the number of
> size and position variables, but I was just curious if you considered it.
This is something I constantly try to do when it reduces code drastically or makes it more readable.
Thanks for the review.
Samuel-