kate2753/flix

flix's Issues

Review

Hey Kate,

I just wanted to capture this feedback in case you wanted to go back through it later:

• Overall, you were following standard good Swift code styling conventions. The Ray Wenderlich Swift Style Guide and the Github Swift Style Guide are two great resources for reading up on Swift coding conventions.
  • In MoviesViewController, some developers will put protocol implementations like UISearchBarDelegate into an extension.
  • In DetailViewController, line 31, you called sizeToFit, which is the right thing to do. Note that you're not allowed to do that if Auto Layout is enabled. Similarly, you can no longer manually modify the frame if you are using Auto Layout.
• In DetailViewController, you added an image to the window. Instead of doing that, you should have created a new view controller to display that image. You can customize the transition of the view controller to be anything you want. Using the window is a last resort thing. A common use case for using the window is if you want to layer things on top of the status bar that displays the time and battery.
• In your MoviesViewController, you were toggling the visibility of a table view and a collection view, which was fine for this assignment. This week, we'll learn how to implement container view controllers which would have allowed you to split that implementation between two view controllers. Alternatively, you could have used a single collection view with two different layouts.
• Using groups to organize your source files. It's pretty common to organize your source files into Groups, especially for larger projects. You can organize by type of class (i.e. View Controllers, Views, etc) or for larger apps by functionality (i.e. Home Screen, Search Screen, etc). You already did this for your project.
• Minimize the number of public functions and properties in your classes. You should always try to minimize the number of properties and functions that are public or internal. If unspecified, the scope is internal.
• Set the Content Mode for your ImageViews. By default the UIContentMode will stretch the image to match the dimensions of the image view, which is probably not what you want. You usually want it set to Aspect Fill with clip subviews, which is what you did in this project.
• Avoid naming collisions inside of your UITableViewCell. When creating a custom cell make sure to never name a property imageView, titleLabel, or subtitleLabel. Those are already defined by UITableViewCell and using them in your custom cell will cause subtle things to happen such as image views not loading until you click on the cell. You didn't do this, but something to watch out for in the future.
• Model classes. Creating a Movie model class will allow you move the deserialization code out of the view controller. You should also create a movie property in your custom cell and use a property observer to update the cell's subviews. There are certain libraries like SwiftyJSON that remove the need for a lot of boilerplate code and make it easier to convert from JSON to your model objects.