Skip to content

Peer Review: Sam Siu#2

Open
ziusziu wants to merge 1 commit intoadao1123:masterfrom
ziusziu:master
Open

Peer Review: Sam Siu#2
ziusziu wants to merge 1 commit intoadao1123:masterfrom
ziusziu:master

Conversation

@ziusziu
Copy link

@ziusziu ziusziu commented May 10, 2016

Does the project appear to meet the technical requirements? Write up one sentence on your findings and give a score 0-3.
I'd give the app a score of 1, considering it's only Tuesday and you've gotten so much done. Minor errors below are:

  1. App crashed on load, it does not ask user to turn on locations.
    After I turn on locations, first time loading app crashes, second time loading app is okay.
  2. App crashes on rating by Boba option
  3. Rating EditText doesn't do anything

Is your peer making API calls, using SDK's/third-party libraries? Is your peer making use of Services? If so, are they offloading long tasks to a separate thread, i.e. AsyncTask, Runnable, IntentService, etc. 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? Is your peer making use of RecyclerView? If so, does it appear to be working correctly ( implementation and otherwise )? 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? 2. 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.

Score of 3:
I like the organization of the app and how there are methods for fragment transactions, such as, passClickedTea(), onBobaSwipeRight(), onDistanceFabClick(), etc. It makes the code easy to follow and read.

Firebase is used, as well as YelpApi, I would put those two into a separate class called "Utils", "DataManager", "ApiCaller" or something similar. Right now their hidden in the ShopActivity and I wouldn't know to look for the API stuff unless I knew your app.

I definitely like the Search menu pop up, it's different, sleek and really nice. Definitely sets you apart!
The only thing that can cause is a problem is when there is a Boba cardview underneath it. Because the color scheme is the same, it's hard to see the options. Maybe make it a ColorPrimaryDark?

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 )? Is your peer making anything cool or awesome that you would like to note or applaud them on? 3. 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.

Score: 3
The pop out menu button is freaking awesome!
When I pop out menu bar in horizontal mode, one of the "by Boba" option gets chopped off.

Are you able to reasonably follow the code without having anyone answer your questions? Are you able to make sense of what the code is doing or is trying to do? 4. 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.

Easy to Follow: Method name explains everything clearly.
@OverRide
public void onBobaSwipeRight() {
fragmentTransaction = fragmentManager.beginTransaction();
homeFragment = new HomeFragment();
fragmentTransaction.replace(R.id.fragment_contatinerID, homeFragment);
fragmentTransaction.commit();
}

Hard to Follow: Mainly because I'm not too familiar with FireBase.
private void initFirebase(){
firebaseRef = new Firebase("https://rate-my-boba.firebaseio.com/");
authData = firebaseRef.getAuth();
String userID = authData.getUid();
userName = (String) authData.getProviderData().get("displayName");
Log.i(TAG, "initFirebase: PRINT NAME " + userName);
firebaseShops = firebaseRef.child("Shops");
firebaseChildShop = firebaseShops.child(teaShop.id());
firebaseChildShop.child("name").setValue(teaShop.name());
firebaseChildShop.child("rating").setValue(teaShop.rating());
//firebaseReviews.setValue(teaShop.reviews().get(0).ratingImageLargeUrl());
Log.i(TAG, "initFirebase: Rating image " + teaShop.ratingImgUrlLarge());
firebaseReviews = firebaseChildShop.child("review");
firebaseReviews.addListenerForSingleValueEvent(new ValueEventListener() {
@OverRide
public void onDataChange(DataSnapshot dataSnapshot) {
Review review = new Review(teaShop.reviews().get(0).user().name(),teaShop.reviews().get(0).excerpt(),teaShop.reviews().get(0).rating()+"");
if (!dataSnapshot.hasChildren()) firebaseReviews.push().setValue(review);
Log.i(TAG, "onDataChange: inside ");
}

        @Override
        public void onCancelled(FirebaseError firebaseError) {

        }
    });
    setReviewRV();

}

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 undertand What makes the difficult code harder to follow? Be as detailed as you can in your answer. 5. High level project overview: Take a look at as many individual files as you have time for

The methods make the code readable, the onCreate() in ShopActivity, tells me exactly what is going on.
initViews();
handleYelpAPI();
setPhoneIntent();
setBobaRV();
submitReviewListener();
However, I'm not to sure about what's going on in the MainActivity. It seems like a container for methods.
locationManager = (LocationManager)getSystemService(Context.LOCATION_SERVICE);
setFragmentManager();

Does this class make sense? Does the structure of the class make sense? Is it clear what this class is supposed to do?

The Fragment Classes are a little confusing. There is a Main, Facebook and Shop Activities. I'm not too sure where Search Fragment links to. I think it deals with the FAB button, but not sure where that logic is.

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