Add main functionality #1
Conversation
There was a problem hiding this comment.
- I believe it would be much better if a descriptive series of commits are considered, afterwards, rather than a single commit.
- Although some classes look very organized others need to be a little bit organized like
GitPopPageFragment&GitPopListFragment. - +1 for using Realm database and some fancy UI.
@AFGhazy Good job! 👍
| Context mContext; | ||
|
|
||
| private static final Map<String, Integer> colors = new HashMap<String, Integer>(){{ | ||
| put("Java", Color.BLUE); |
There was a problem hiding this comment.
@AFGhazy I suggest using constants, in general, rather than static strings to improve readability and maintainability. What do you think?
| * Created by ahmedghazy on 7/17/18. | ||
| */ | ||
|
|
||
| public class GitPopListActivity extends SingleFragmentActivity { |
There was a problem hiding this comment.
@AFGhazy I'm not sure but this class seems to be never called in the code base. Kindly correct me if I missed something.
UPD
Sorry I missed that it's the launcher
| Retrofit mRetrofit; | ||
| GitPopService mGitPopService; | ||
| public GitPopFetcher() { | ||
| // HttpLoggingInterceptor logging = new HttpLoggingInterceptor(); |
There was a problem hiding this comment.
@AFGhazy IMHO It will be better to do without commented lines of code. What do you think?
| @@ -0,0 +1,24 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
@AFGhazy +1 For using layer list drawables.
| } | ||
| } | ||
|
|
||
| public class ContributerHolder extends RecyclerView.ViewHolder implements View.OnClickListener { |
There was a problem hiding this comment.
@AFGhazy I vote for defining this class in a standalone .java file rather than nesting it in another one.
| } | ||
|
|
||
| @Override | ||
| public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { |
There was a problem hiding this comment.
@AFGhazy I believe it would be better if we can consider smaller methods bodies afterwards.
| private String html_url; | ||
| private String language; | ||
| private String id; | ||
| private String full_name; |
There was a problem hiding this comment.
@AFGhazy Kindly consider the java camel case convention afterwards.
| */ | ||
|
|
||
| public class GitPopListFragment extends Fragment implements Callback<List<Repo>> { | ||
| @BindView(R.id.git_pop_recycler_view) RecyclerView mGitPopRecyclerView; |
There was a problem hiding this comment.
@AFGhazy try to use the same code conventions, BindView and variable in same line or on separate lines
| @BindView(R.id.git_pop_recycler_view) RecyclerView mGitPopRecyclerView; | ||
| @BindView(R.id.empty_view) | ||
| TextView mEmptyView; | ||
| GitPopAdapter mGitPopAdapter; |
There was a problem hiding this comment.
@AFGhazy you can make them private
|
|
||
| @Override | ||
| public void onCreate(@Nullable Bundle savedInstanceState) { | ||
| super.onCreate(savedInstanceState); |
There was a problem hiding this comment.
@AFGhazy don't leave many empty lines inside a method
| } | ||
|
|
||
| @Override | ||
| public void onResume() { |
There was a problem hiding this comment.
@AFGhazy it can be removed as no code is done more than calling super
| } | ||
|
|
||
| @Override | ||
|
|
There was a problem hiding this comment.
@AFGhazy remove empty line
| @Override | ||
| public void onResponse(Call<List<Repo>> call, final Response<List<Repo>> response) { | ||
|
|
||
| if(response.body() != null && response.body().size() > 0 && mRealm.where(Repo.class).equalTo("id", response.body().get(0).getId()).findAll().size() == 0) { |
There was a problem hiding this comment.
@AFGhazy it is better to break this long check to several lines,
move response.body().get(0).getId() to a separate line for example
3 participants
No description provided.