Skip to content

[Intro to iOS] Week 2 Project for Review#3

Open
CMUboy wants to merge 3 commits intocodepath:masterfrom
CMUboy:master
Open

[Intro to iOS] Week 2 Project for Review#3
CMUboy wants to merge 3 commits intocodepath:masterfrom
CMUboy:master

Conversation

@CMUboy
Copy link

@CMUboy CMUboy commented Feb 16, 2015

I am unable to create an issue. I think it's because I forked this repository.

My app is now complete. Please review my app. /cc @codepath /cc @thecodepath

@codepathreview
Copy link

Hey Joseph,

For future assignments, open an issue on your own repo.

👍 nice work. The primary challenge of this homework was to learn how to use UITableView to manage a complex form, establish communication between view controllers, and apply the correct Auto Layout constraints. For each point in the homework feedback checklist, I'll either confirm that you're on the right track with the implementation, or I'll provide some feedback:

  • For the restaurant cell, note that the subviews should be private and only updated in the Swift property observer.
  • You should always specify Auto Layout constraints in your nibs.
  • Note that you should resolve all the Auto Layout constraint warnings in your nibs.
  • It's acceptable to avoid dequeuing the cells, especially in small forms. However, it is still good practice to link the form with the underlying model. See this popular form library for an alternative to implementing forms manually.
  • Your filter cells aren't tracking their states properly. If you scroll them off the page, you can see that the selection status isn't being restored properly.
  • Connecting the filter view controller directly to a global filter settings is convenient, but the downside is if you cancel the filters, the values have already been saved. Consider defining a custom protocol to capture the filter change event.

This was a challenging assignment, congrats on completing it successfully!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants