HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Task Reminder - Repeatedly notify every 6 hours

Submitted by: @import:stackexchange-codereview··
0
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 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 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.