patternjavaMinor
Task Reminder - Repeatedly notify every 6 hours
Viewed 0 times
notifyhourseveryreminderrepeatedlytask
Problem
So I'm very much a beginner to Java and Android programming. I've worked a lot more with Python so I might have some quirks from there bleeding into my code here. I'd like a particular focus on pointing out ways I'm approaching the language and any confusing style I might be employing.
What the app does is pretty simple. The user enters a task and then the app will regularly notify them a reminder to do the task if they haven't already. When the task is done, the user can enter the app, clear the task and set a new one. There's just one screen that has an EditText and two buttons, one for submitting a task and one for clearing it.
I'm not planning to expand it a lot, the intention is to be simple. But I will add settings, to change how long between notifications and whether notifications should have sounds or vibrate.
In my testing it seems that the timing of notifications is erratic, but the documentation says AlarmManagers are just like that. Should I trust this since I don't need a precise interval anyway?
Here's the full code, my
MainActivity
```
public class MainActivity extends AppCompatActivity {
private EditText enterTask;
private Button button;
private Button clear_task;
private String task;
private SharedPreferences prefs;
private final long SIX_HOURS = 21600000L;
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
prefs = PreferenceManager.getDefaultSharedPreferences(this);
enterTask = (EditText) findViewById(R.id.enter_task);
button = (Button) findViewById(R.id.submit);
clear_task = (Button) findViewById(R.id.clear_task);
// Try get the current task String.
task = (prefs.getString("Current_Task", ""));
if (!task.equals("")) {
Toast.makeText(getApplicationContext(),
What the app does is pretty simple. The user enters a task and then the app will regularly notify them a reminder to do the task if they haven't already. When the task is done, the user can enter the app, clear the task and set a new one. There's just one screen that has an EditText and two buttons, one for submitting a task and one for clearing it.
I'm not planning to expand it a lot, the intention is to be simple. But I will add settings, to change how long between notifications and whether notifications should have sounds or vibrate.
In my testing it seems that the timing of notifications is erratic, but the documentation says AlarmManagers are just like that. Should I trust this since I don't need a precise interval anyway?
Here's the full code, my
MainActivity as well as my BroadcastReceiver and Service.MainActivity
```
public class MainActivity extends AppCompatActivity {
private EditText enterTask;
private Button button;
private Button clear_task;
private String task;
private SharedPreferences prefs;
private final long SIX_HOURS = 21600000L;
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
prefs = PreferenceManager.getDefaultSharedPreferences(this);
enterTask = (EditText) findViewById(R.id.enter_task);
button = (Button) findViewById(R.id.submit);
clear_task = (Button) findViewById(R.id.clear_task);
// Try get the current task String.
task = (prefs.getString("Current_Task", ""));
if (!task.equals("")) {
Toast.makeText(getApplicationContext(),
Solution
Variables that can be local
All member variables in
As such, they can all be converted to local variables.
It's recommended to define variables in the smallest scope necessary.
In case of member variables,
you need to read the entire class to understand all their uses.
The same is true for
except the
and
which can be left where they are.
Empty string?
Instead of
Empty constructors
I'm wondering if you really need the empty constructors,
for example in
If they are not required, I suggest to remove them.
Naming
In Java,
variables should be named
which is violated by
It's especially odd that it's inconsistent with
just a few lines away.
In Android development it's a common practice to prefix member fields with
I'm not a big fan of that,
but it's ok if you like it.
However, either use it consistently everywhere,
or use
and
I suggest to just spell these out.
Other variables could also have better names.
It would be better to name it according to the purpose it represents.
All member variables in
NotifyAlarm are only used in one method.As such, they can all be converted to local variables.
It's recommended to define variables in the smallest scope necessary.
In case of member variables,
you need to read the entire class to understand all their uses.
The same is true for
MainActivity,except the
SIX_HOURS constant,and
task,which can be left where they are.
Empty string?
Instead of
!task.equals(""), the idiomatic way to check if a string is non-empty is using !task.isEmpty().Empty constructors
I'm wondering if you really need the empty constructors,
for example in
Notify and NotifyAlarm.If they are not required, I suggest to remove them.
Naming
In Java,
variables should be named
camelCase,which is violated by
clear_task.It's especially odd that it's inconsistent with
enterTask,just a few lines away.
In Android development it's a common practice to prefix member fields with
m.I'm not a big fan of that,
but it's ok if you like it.
However, either use it consistently everywhere,
or use
camelCase consistently everywhere.i is a very poor name for an Intent,and
pi for a PendingIndent.I suggest to just spell these out.
Other variables could also have better names.
SIX_HOURS is literally what it means.It would be better to name it according to the purpose it represents.
Context
StackExchange Code Review Q#104537, answer score: 4
Revisions (0)
No revisions yet.