Skip to content

Instantly share code, notes, and snippets.

@AdrianBinDC
Last active June 14, 2020 15:35
Show Gist options
  • Save AdrianBinDC/2a6d17d4c0894276df6bca802cd915dc to your computer and use it in GitHub Desktop.
Save AdrianBinDC/2a6d17d4c0894276df6bca802cd915dc to your computer and use it in GitHub Desktop.

Issues

CocoaPod Bloat

  • There's a ton of CocoaPods in the project. Do you need them all?
    • I see two types of cocoapods for image caching

Lack of Tests

  • there are only tests for one class and they don't pass
  • I should be able to download the repo, install the cocoapods and run the tests and they should all pass without having to jump through special hoops.me.com

Fit & Finish

Color Theme

  • Pick a color scheme, define it in a central location, and use it throughout the app on the UI

Signin Screen

  • signin screen capitalizes first letter
  • login screen capitalizes first letter

FirstProfileViewController

  • Most of the issues w/ this view controller would go away with a stack view
    • Refactor to a stackview
  • User Name is empty
  • User Type doesn't show anything
  • User Rating doesn't show anything
  • Phone Number is blank
  • Email address is cut off

ProfileViewController

  • The hittable area for the button at the top does not meet the Human Interface Guidelines for iOS.
  • Re-implement the button with an image where the whole thing is a button instead of an itty bitty target.
  • The camera icon is misleading because it gives the impression the user can use the camera, which is not one of the options presented on the share sheet.
  • There’s a typo for Photo Library on the share sheet
  • There’s no “Done” button or “Submit” button once the user is finished typing stuff in.
  • There’s no validation for entering text in the textfields.
  • Number textFields should open a number keypad and not allow letters

RootViewController

  • When map loads, it would be cool if it loaded with data populated
  • Here's a quick summary of how to set a center, a region, etc. on a MapView
  • When a cell is tapped on CardViewController, it would be cool if there were a map button that would re-center the map on RootViewController.
  • Add logic so the map doesn't throw an exception if too many posts are loaded.
    • Use breakpoints to determine where that logic should go

Localization

  • Localization might be something to consider once existing issues with the repo are resolved. For now, would leave it alone, but down the road, you guys could do some cool stuff with Russian and Ukrainian at a minimum.

Code Style

  • Swift is an expressive language. As such, make use of that in declarations. Instead of cv, it's prefereable to declare as collectionView.
  • When you work in a larger environment with a lot of people on the team, you'll definitely appreciate unambiguous, expressive code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment