Skip to content

Codereview #2 of 8 from 3#104

Open
leopardo-rossi-developer wants to merge 9 commits intoPrandiniUniPD:CommonDemofrom
leopardo-rossi-developer:codereview2_second_reviewer
Open

Codereview #2 of 8 from 3#104
leopardo-rossi-developer wants to merge 9 commits intoPrandiniUniPD:CommonDemofrom
leopardo-rossi-developer:codereview2_second_reviewer

Conversation

@leopardo-rossi-developer
Copy link
Copy Markdown
Collaborator

With my group we're developing a forum for the app, where users can read useful information/article published by other users and let them know (using comments and likes) if they appreciate what is written. All this information is stored into a DB handled thanks to Firebase Firestore API.

In particular the class you have to review is the one that manages:
1- Posts download from DB.
2- Like addition to a post.

Copy link
Copy Markdown
Collaborator

@thomasporro thomasporro left a comment

Choose a reason for hiding this comment

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

The code is really nice to read. The only thing, you might follow more the style doc, for example the column limit of 100 characters. I added a comment in a line that I don't understand too much

import unipd.se18.ocrcamera.forum.R;
import unipd.se18.ocrcamera.forum.RequestManager;
import unipd.se18.ocrcamera.forum.models.Post;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

According to our style document:

If there are both static and non-static imports, a single blank line separates the two blocks. There are no other blank lines between import statements.

* According to the MVVM (Model-View-ViewModel) architecture this class contains all fragment ShowPosts' logic.
* This architecture ensures that the code is more testable and organised than the classic approch to put everything
* in the activity or fragment class.
* In particular the methods that are implemented here are used to get posts from forum and to handle users' likes. The
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a distraction error: users' instead of user's

//If an error occurs while converting the post's date an error message is logged to console and
//the failure UI listener is triggered with a explanation message for the user
Log.d(LOG_TAG, e.getMessage());
if (getPostListener != null){ getPostListener.onGetPostFailure(context.getString(R.string.requestFailedMessage)); }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it's a my problem beacause I've never used the listeners but I don't understand where the getPostListener is initialized.

public void onSuccess(Object o) {
//If the like has correctly been added to the db the success listener is triggerd
//(In this case there's an empty string as parameter because there's no specific message to show to the user)
if (addLikeListener != null) { addLikeListener.onAddLikeSuccess(""); }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The same thing as above

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.

3 participants