Skip to content

Created peer_review.md#1

Open
cherence wants to merge 1 commit intoblakepolidore:masterfrom
cherence:master
Open

Created peer_review.md#1
cherence wants to merge 1 commit intoblakepolidore:masterfrom
cherence:master

Conversation

@cherence
Copy link

@cherence cherence commented May 10, 2016

Questions to answer:

  1. Does the project appear to meet the technical requirements? Write up one sentence on your findings and give a score 0-3. 3. Your project is very ambitious and I applaud your use of so many API calls/SDK's/third-party libraries.

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.

  1. Does the project appear to be creative, innovative, and different from any competition? Write up one sentence on your findings and give a score 0-3. 2 The swipe functionality is trendy and and innovative way to help users pick an activity.

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.

  1. Does the project appear to follow correct coding styles and best practices? Write up one sentence on your findings and give a score 0-3. 3 Your code is so well organized--from the package folders to the variable regions to the java docs.

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.

  1. Find two pieces of code of any size: one that is readable and easy to follow and one that is difficult to follow and understand.

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.

  1. High level project overview: Take a look at as many individual files as you have time for

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.

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