-
Notifications
You must be signed in to change notification settings - Fork 29
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
assignments submitted #32
base: main
Are you sure you want to change the base?
Conversation
@Override | ||
public void onBindViewHolder(@NonNull final ViewHolder holder, int position) { | ||
|
||
final Article article = articles.get(position); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a null check for the article
|
||
holder.cardView.setOnClickListener(view -> { | ||
Intent intent = new Intent(context, DetailedActivity.class); | ||
intent.putExtra("title", article.getTitle()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move all the strings as constants
intent.putExtra("desc", article.getDescription()); | ||
intent.putExtra("imageUrl", article.getUrlToImage()); | ||
intent.putExtra("url", article.getUrl()); | ||
context.startActivity(intent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add startActivity in try catch block because in case there is no activity found for the corresponding intent, then it will crash
} | ||
|
||
public static class ViewHolder extends RecyclerView.ViewHolder { | ||
TextView tvTitle,tvSource,tvDate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define the scope of the variables properly, certain variables are getting used locally only so need not to be defined at member level
loader.setVisibility(View.VISIBLE); | ||
|
||
Intent intent = getIntent(); | ||
String title = intent.getStringExtra("title"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use constants instead of directly using strings
protected void onCreate(Bundle savedInstanceState) { | ||
super.onCreate(savedInstanceState); | ||
|
||
com.example.news.databinding.ActivityMainBinding binding = ActivityMainBinding.inflate(getLayoutInflater()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to define whole package when import is already there
setSupportActionBar(binding.appBarMain.toolbar); | ||
|
||
binding.appBarMain.fab.setOnClickListener(view -> | ||
Snackbar.make(view, "Replace with your own action", Snackbar.LENGTH_LONG).setAction("Action", null).show()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define all strings in strings.xml
} | ||
|
||
@Override | ||
public boolean onCreateOptionsMenu(Menu menu) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is no menu item, then this code can be removed, Clean the code as much as possible
public class ExampleUnitTest { | ||
@Test | ||
public void addition_isCorrect() { | ||
assertEquals(4, 2 + 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this test case about ?
|
No description provided.