Tang sarah #7
Tang sarah #7
Conversation
| String host = intent.getStringExtra(SocialsAdapter.HOST_KEY); | ||
| String description = intent.getStringExtra(SocialsAdapter.DESCRIPTION_KEY); | ||
| String image = intent.getStringExtra(SocialsAdapter.FIREBASE_URL); | ||
| final int numberInterested = intent.getIntExtra(SocialsAdapter.NUMBER_INTERESTED, -1); |
There was a problem hiding this comment.
What is -1, I can't tell (is it the default value?). Specify in a final variable what this is. Maybe call it DEFAULT_INTERESTED so it's easier to read and easier to change later.
| firebaseKey = intent.getStringExtra(SocialsAdapter.FIREBASE_KEY); | ||
|
|
||
| //Download image asynchronously | ||
| class DownloadFilesTask extends AsyncTask<String, Void, Bitmap> { |
There was a problem hiding this comment.
This could be defines outside the onCreate and wouldn't change anything except code readability
|
|
||
|
|
||
| //Retrieving image from firebase storage | ||
| FirebaseStorage.getInstance().getReferenceFromUrl("gs://mdbsocials-fdfae.appspot.com").child(image + ".png").getDownloadUrl().addOnSuccessListener(new OnSuccessListener<Uri>() { |
| descriptionDetail.setText(getString(R.string.descriptionIntro) + description); | ||
|
|
||
| switch(numberInterested) { | ||
| case 1: |
There was a problem hiding this comment.
While switch cases are nice, there's really no point if you're doing a simple if else thing. Switch case is more for when you have 3 or more cases. Not really a problem, just don't feel pressured to always use a switch case
|
|
||
| public static void addInterestedToDatabase(int interested, String firekey) { | ||
| DatabaseReference ref = FirebaseDatabase.getInstance().getReference("/socials"); | ||
| ref.child(firekey).child("numberInterested").setValue(interested); |
There was a problem hiding this comment.
These types of strings are important to keep as final variables to ensure consistency. You can get a String wrong in another class, but you can't get a variable name wrong and still compile
| //Connect people interested in social with recycler view layout | ||
| Log.d("InterestedActivity", "Int"); | ||
| RecyclerView recyclerViewInterested = (RecyclerView) findViewById(R.id.recyclerInterested); | ||
| recyclerViewInterested.setLayoutManager(new LinearLayoutManager(getApplicationContext())); |
There was a problem hiding this comment.
the this keyword is alright, though you can use getApplicationContext() if you really want to be precise
| */ | ||
|
|
||
| mAuthListener = new FirebaseAuth.AuthStateListener() { | ||
| @Override |
|
|
||
| @Override | ||
| public void onDestroy() { | ||
| super.onDestroy(); |
There was a problem hiding this comment.
onDestroy also calls onStop, no need to repeat this code
| .setPositiveButton("Camera", new DialogInterface.OnClickListener() { | ||
| public void onClick(DialogInterface dialog, int id) { | ||
| Intent cameraIntent = new Intent(MediaStore.ACTION_IMAGE_CAPTURE); | ||
| startActivityForResult(cameraIntent, 1); |
There was a problem hiding this comment.
1 should be a final variable called CAMERA_CODE or something
| public static final String PEOPLE_INTERESTED = "peopleInterested"; | ||
| public static final String NUMBER_INTERESTED = "numberInterested"; | ||
| public static final String FIREBASE_URL = "firebaseURL"; | ||
| public static final String FIREBASE_KEY = "firebaseKey"; |
There was a problem hiding this comment.
you use these keys in other classes too, better to just put them in a Utils class
Fix