Conversation
|
@blakepolidore Overall this is an excellent project. Code quality is awesome, you have great structure. Lots of packages that make sense. I would make a top level Lower score related to gitHub is that I saw no evidence of branching. Usually I'd see at least a few commits stating "Merging branch X into branch Y" in the commit logs. You did a great job with commits themselves and I can definitely follow along and see the progress. The feature branches just make your life easier and reflect a real word work environment. Even during solo development is pays off to use branches so I'd encourage you to try. If you did branch and the commits don't reflect that, let me know we can update the score. On a higher level, I ran your application on my GenyMotion emulator instead of a real phone. Just to change things up. Ran into a few bugs and UX issues.
This way if the play services is not available, then you can prevent crashes and either ask user to install it or change up their in app experience. This is not a bug I expected you to find/test but the point here is that its important to test on more then one device. Tablets from class, your phone, emulators, etc. Since Android has lots of fragmentation and the "it works on my phone" argument does not roll well in a work environment when you're expected to support many devices. This is the reason for lower score in the handling errors, etc category of project. Its tough to test it all but as developers we need to make time to test the things we build and not rely heavily on QA or users to do it for us as they are bigger stakeholders then us. Project feedback: Main3Activity.java is very long, try to think of ways to make it shorter. You can move some of the Api callbacks into classes, along with click listeners, etc. Or perhaps a manager class to manage state of things and move them out of Main3Activity. This way, this manager could be instantiated in another activity and re-used. CardModelAndAdapter package is doing more then one thing. Just use an adapter package and hold the adapter there. CardModel could go under models package I noted above but maybe not within any subpackage. Same for your forsquare adapters. It would go into the above Adapter package, but your click listeners and interfaces should go into their own packages that make sense. Same for swipeFling package. Idea with packages is to group related things such as all adapters in one place, all listeners or helper classes, etc. This makes navigating the project easier, adding new types of these objects is simple and you know where to put them. It ups the organization to a whole new level. You do not have any Unit Tests or Instrumentation Tests. I highly encourage you to write some, they look great for employers but more importantly they test your code. And they force you think about the classes you are designing because its one thing to make a class, its another to make one that is testable! |
Project 4 | ADI
Performance Evaluation
Score:
Based on the requirements, you can earn a maximum of 18 points on this project.
Your total score is: 18
@blakepolidore Great job, solid looking project! UI looks great, code quality is awesome!