Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Questions to answer:
Is your peer making API calls, using SDK's/third-party libraries? Yes, Foursquare, RottenTomatoes, Swipecards, Retrofit2, Yelp, Butterknife, Picasso, ImageLoader, Timber, Facebook, Firebase, and GooglePlay Services.
Is your peer making use of Services? If so, are they offloading long tasks to a separate thread, i.e. AsyncTask, Runnable, IntentService, etc. Yes, API and network calls are made in background threads. But there is a lag loading photos.
Is your peer making use of Fragments? If so, are they passing data from Fragment to Activity via interfaces? If not, why did absense of Fragments make sense? No fragments. Absence of fragments makes sense the UI is DRY.
Is your peer making use of RecyclerView? If so, does it appear to be working correctly ( implementation and otherwise )? Yes, list of favorites scrolls seamlessly.
Is your peer making use of some sort of persistent storage, i.e. Firebase or SQLite? If so, why do you think Firebase/SQLite was chosen? Could they have used one or the other instead and why? Yes, Firebase. Chosen to utilize live updates of user likes.
Is your peer making use of proper UX patterns we learned in class? If not, what are they doing that is unconvetional or that might confuse a user ( you )? I only knew to swipe because you told me. It would be cool if there was a quick tutorial splash screen or dialog box.
Is your peer making anything cool or awesome that you would like to note or applaud them on? Love the navigation drawer and the toggle buttons. The sliding scale wasn't the easiest to use (drawer would close when I wanted to reduce the search radius.
Are you able to reasonably follow the code without having anyone answer your questions? Yes, it is well documented with comments and java docs.
Are you able to make sense of what the code is doing or is trying to do? Yes, it really helps that your variables and methods follow java/android naming conventions.
What makes the readable code readable? Be as detailed as you can in your answer, it can be challenging to explain why something is easy to understand.
private void setViews() {
flingContainer = (SwipeFlingAdapterView) findViewById(R.id.frame);
navigationView = (NavigationView) findViewById(R.id.nav_view);
scrollView = (ScrollView) navigationView.findViewById(R.id.nav_scrollview);
locationEditText = (EditText) scrollView.findViewById(R.id.location_editText);
userQueryEditText = (EditText) scrollView.findViewById(R.id.userQuery_editText);
radiusSeekbar = (SeekBar) scrollView.findViewById(R.id.search_radius_seekbar);
deviceLocationSwitch = (Switch) scrollView.findViewById(R.id.phone_location_switch);
foodSwitch = (Switch) scrollView.findViewById(R.id.food_search_switch);
drinkSwitch = (Switch) scrollView.findViewById(R.id.drink_search_switch);
locationsSwitch = (Switch) scrollView.findViewById(R.id.activities_search_switch);
eventsSwitch = (Switch) scrollView.findViewById(R.id.events_search_switch);
logOut = (Button) scrollView.findViewById(R.id.logoutButton);
seekbarProgress = (TextView) scrollView.findViewById(R.id.seekbar_progress);
dislikeButton = (ImageButton) findViewById(R.id.dislikeButton);
likeButton = (ImageButton) findViewById(R.id.likeButton);
drawerLayout = (DrawerLayout) findViewById(R.id.drawer_layout);
}
I love that the type was included int he name. That way, when the view element is referenced you know exactly which view you’re referring to.
What makes the difficult code harder to follow? Be as detailed as you can in your answer.
private boolean resetCardViewOnStack() {
if (movedBeyondLeftBorder()) {
// Left Swipe
onSelected(true, getExitPoint(-objectW), 100);
mFlingListener.onScroll(-1.0f);
} else if (movedBeyondRightBorder()) {
// Right Swipe
onSelected(false, getExitPoint(parentWidth), 100);
mFlingListener.onScroll(1.0f);
} else {
float abslMoveDistance = Math.abs(aPosX - objectX);
aPosX = 0;
aPosY = 0;
aDownTouchX = 0;
aDownTouchY = 0;
frame.animate()
.setDuration(200)
.setInterpolator(new OvershootInterpolator(1.5f))
.x(objectX)
.y(objectY)
.rotation(0);
mFlingListener.onScroll(0.0f);
if (abslMoveDistance < 4.0) {
mFlingListener.onClick(dataObject);
}
}
return false;
}
This block of code was a little more difficult to follow. It would be helpful if you added java docs and explained what the variables are and do. Ex: aPosx, aPosY, aDownTouchX, and aDownTouchY.
Movies object model.
Does this class make sense? Yes
Does the structure of the class make sense? Yes
Is it clear what this class is supposed to do? Yes, models the JSON object returned from the API request.