Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sumukh #2

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Sumukh #2

wants to merge 16 commits into from

Conversation

sumukhshiv
Copy link

Thoughts on the project - what was liked? What could have gone better?

Project was nice. It was great to look back on a pretty large project and fix bugs and modularize a lot more. I really want to back to this project once more and fix some more UI elements. Pretty happy with the project overall. On the whole nothing went too bad on this project. Extracting string resources was just annoying.

How long was spent working on the project?

I spent about 8 hours on this project. Most of the time was fixing old bugs from the last submission and rewriting sections for better modularity.

super.onResume();

//grabbing the social, which was just clicked on from UserArea
Intent intent = getIntent();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you use this intent anywhere else? If not, don't define it as a variable.

Toast.makeText(context, "Please enter a valid Email Address", Toast.LENGTH_SHORT).show();
}
else if (mAuth.signInWithEmailAndPassword(email, password) != null) {
Toast.makeText(context, "An account with this email already exists.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firebase already returns errors like this, so don't try to catch it on your own. Just display Firebase's error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, the task is not successful but it does not return anything. How would I go about finding why the task failed. I had it this way, so that the user at least has some insight as to why he/she was not able to register.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

task.getException() returns the error

recyclerView.setLayoutManager(new LinearLayoutManager(getApplicationContext()));

//Grab social objects people interested arraylist for adapter
profilesInterested = social.peopleInterested;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be a separate value? Does this variable need to be instantiated?

public void onClick(View view) {
switch (view.getId()) {
case R.id.buttonSignin:
userLogin();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good modularization by having a userLogin() function

break;

//Posting the Social Firebase and Newsfeed
case (R.id.buttonPost):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having so much code below this case makes it hard to read. I recommend modularizing it into a function

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.

2 participants