Skip to content

Project 1 Feedback#1

Open
aleksandrTmk wants to merge 1 commit intoStarAceM:masterfrom
ADI-SF-Student-Projects:master
Open

Project 1 Feedback#1
aleksandrTmk wants to merge 1 commit intoStarAceM:masterfrom
ADI-SF-Student-Projects:master

Conversation

@aleksandrTmk
Copy link

RUBRIC

Project 1 | ADI

Performance Evaluation

Mark boxes with an 'X'; if did not achieve bonus, mark with a '-'

Requirements Incomplete (0) Does Not Meet Expectations (1) Meets Expectations (2) Exceeds Expectations (3)
READ.me is created x n/a
View a collection of to-do lists x n/a
View items on a to-do list x n/a
Allow the user to create a new to-do list x n/a
Add items to each to-do list x n/a
Display correctly in both landscape and portrait orientations x
Bonus: Show an error message if invalid input is given n/a x n/a x
Bonus: Allow the user to check off and remove completed items n/a n/a n/a n/a
Bonus: Add an item detail screen that allows the user to give an optional description for each item n/a n/a n/a n/a
Bonus: Delete master list items n/a x n/a n/a
Bonus: Delete sub list items n/a n/a n/a x

Notes:
Great job with completing the bonuses and going above and beyond the requirements! Keep it up. Nice work handling the screen orientation changes.


Score:

Based on the requirements, you can earn a maximum of 12 points on this project.

Your total score is: 13


@aleksandrTmk
Copy link
Author

@mstarace Great work on this project!

  • Nice job with the orientation changes!
  • Good job handling user input!
  • I like the attempt to use classes within the project!

Overall you did a good job with the project! Take a look at some feedback below to improve yourself as a developer:

  • The classes Item.java and DataHandler.java are empty/commented out. In such cases, it is best practice to completely remove them from the project instead of keeping them around.

MainActivity.java review:

  • onCreate() is short and concise. Great job breaking things up into specific methods that handle small jobs! It would be ideal if you removed all the extra white space from newlines. For examples, lines 53-66 could remove 6 newlines which would make that for loop that much shorter. The same goes for the rest of the class.
  • Good job with figuring out the onSaveInstanceState()! One thing to note is that it is best practice to take your strings, such as the "change" in outState.putStringArrayList("change", todoList); on line 199 and turn it into a public static final String SAVED_TODO_LIST = "change";. Then calling outState.putStringArrayList(SAVED_TODO_LIST, todoList);. The same goes for lines 55, 57, 136-138, 157, and 207.
  • BUG: I can input lists with no name! You should make sure that users have entered a name before creating a list!
  • I was able to crash the app on line 137 with crash java.lang.IndexOutOfBoundsException: Invalid index 2, size is 1. This is related to the bug you described with deleting items. Need to check bounds before calling the get() method in line 137. Seems like your onItemLongClickListener() and onItemClickListener() are triggering right after each other. The reason for this is that in line 188 you are returning false when you should be returning true. Reading documentation for onItemLongClickListener() -> onItemLongClick() method, we see that Return true if the callback consumed the long click, false otherwise which means that if we return false, the click is NOT consumed, and thus you get the onItemClick() callback being activated. By returning true, you say that you consumed the touch event, i.e. you handled it and it shouldn't be sent for further processing.

DetailActivity.java review:

  • Great job breaking things up into many methods that do their specific jobs and nothing else.
  • Please remove extra white space newlines throughout the class.
  • line 64 uses a hardcoded string when it should be using a constant. If inside of MainActivity.java you declared a constant public static final String KEY_DATA_LIST = "dataList"; and used it throughout MainActivity.java, you could have also done on line 64 inside DetailActivity.java a simple return fromMain.getStringArrayListExtra(MainActivity.KEY_DATA_LIST);.

Overall good job on the project. Please consider taking care of user input in ALL cases, such as on the main screen. I hope it becomes more clear to you why your delete item bug existed.

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.

1 participant