Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thanks! #1

Open
coderschoolreview opened this issue Jun 24, 2016 · 1 comment
Open

Thanks! #1

coderschoolreview opened this issue Jun 24, 2016 · 1 comment

Comments

@coderschoolreview
Copy link

Thanks for your submission. Everything looks good with that. We'll review it and get back to you soon :)

@avo1
Copy link

avo1 commented Jun 28, 2016

Hi Quang Huy,

Thanks for submission. Good job with Tip Calculator 👍

Quick Comments:

  • NSUserDefaults is global. You don't need to pass it from one screen to another screen. This is one of the methods to pass the data from multiple viewController. We will learn few more ways to do that.
  • When you go back from Setting page, you load the default tip percentage in viewDidAppear. Think of it, should it be viewViewAppear? You want to load everything before it shows up to the user.
  • I see you remember the data after app restart, but you haven't implemented the 10 mins, have you? Look at the NSDate() to calculate the time. Think of the reason why we ask to save only within 10min.
  • You shouldn't let the user enter invalid number, should you? I can enter bunch of leading zeros and decimal points. And when your app first run, it show 0 in the bill amount. When I keep typing there will be a leading zero.
  • Nice animation in setting page.
  • Instead of do ... catch ... you can use if let to ensure your data is not nil before you can do something with it.
  • In you notice in your code, xcode reminds few times to use let instead of var for the constant.
    screen shot 2016-06-28 at 7 22 41 pm

Regards,
Dave

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

No branches or pull requests

2 participants