PunithaAnandan / SplitWise

0 stars 1 forks source link

Design Code Review #2

Open Amatheia opened 7 years ago

Amatheia commented 7 years ago

Design/Code Review 1

Project: SplitWise

Developer: Puni

Reviewer: April

Category Criteria Rating/Comments
Problem Statement Does it exist? Yes
Does the problem statement accurately describe project purpose? Yes
Is the problem statement professional? Think of prospective employers viewing this as part of the developer's portfolio. Grammar check, i.e. "in a busy days". Needs better copy.
What is good? Great idea for expense tracking
What could be improved? add expense examples - what kind of bills, other expenses, will be included?
Design Documentation Does it exist? Yes
Is the navigation/flow through the application logical and easy to use? It was hard at first to figure out, but was informed each page is numbered.
Is the order in which the fields are displayed and form fields entered logical and easy to use? Not sure about "Name of your expense" on a sign up page - seems like this is entered later; also address, phone number - not sure why these are necessary fields when recording finances
What data is missing? username, option to add more expenses on "Enter Your Expenses"
Is there data that is not used? View/Edit Expenses - fields are missing or if they appear after selecting month, make a note of the fields/info that will appear
What is good? option on sign in page for forgotten password; back button
What could be improved? add all pages together into PDF for better understanding it comes together; include flow diagram for how the pages connect to each other; turn Main Menu into Navigation Menu; remove Go button on CompareYearOrMonth and turn options into buttons; reset password should be changed to "change password"
Data model Does it exist? No
Is everything on the screens represented in the model?
Does the model represent good database design?
What is good?
What could be better?
Additional design documents No
Application structure in IntelliJ Does it exist? Yes
Is the structure correct for a Maven project? no resources folder
Are packages and classes appropriately named? not sure about this - missing folders - just have servlet names
Other comments/notes?
JSPs Do they exist? Yes
Is templating used (for example, header.jsp, footer.jsp, etc.)? Yes
Is there business logic mixed in the JSPs? Yes
CSS? Yes
Other comments/notes?
Entities/DAOs/Controllers Do they exist? I see one folder for entity, nothing for DAO or controller
Java code quality Are methods single-purpose? N/A
Are classes appropriately-sized classes (no monster classes)? please see note about missing folders
Are the same lines of code repeated at all?
Do any classes perform very similar functions that could be candidates for super/subclass relationships?
Are any values hard-coded that should be in a properties file?
Are variable names descriptive?
Are there many branches or loops that could be simplified or broken up into smaller methods?
Do the DAOs use Hibernate? No hard-coded sql! no resources folder
Other comments/notes?
Logging Has log4J been added? Yes
Are there logging statements in the code? Yes
Are appropriate logging levels used? Info, debug, error, for example. Could use more.
Are there any System.out.printlns in the code? No
Other comments/notes?
Unit Tests Do they exist? No
Do the tests pass?
What is the current code coverage?
Is each test truly a unit test or are they functional tests?
Other comments/notes?
Web Service/API integration Has a web service/api been selected? No
What web services/apis might work well with this application? https://www.programmableweb.com
Independent research topic Has a topic been selected? Yes
What topic/s might fit well in this application? Struts
pawaitemadisoncollege commented 7 years ago

The feedback lost some of its table formatting - can you edit?

Amatheia commented 7 years ago

Fixed.

pawaitemadisoncollege commented 7 years ago

Thanks for fixing formatting.

There are few empty boxes here - if something doesn't apply, can you indicate it as such? N/A if Unit tests have not been added yet, for example.

Thanks!

Amatheia commented 7 years ago

Updated.