5
\$\begingroup\$

I'm building an alarm set to reoccur every 30 days. I was wondering if I could get a few extra sets of eyes to look things over and see if you can notice anything I may have done wrong. Any/all input is greatly appreciated!

SOURCE:

public class WifiMonitor extends Activity {

    Button sendButton;

    EditText msgTextField;

    private PendingIntent pendingIntent;

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        TextView infoView = (TextView) findViewById(R.id.traffic_info);

        double totalBytes = (double) TrafficStats.getTotalRxBytes()
                + TrafficStats.getTotalTxBytes();
        double mobileBytes = TrafficStats.getMobileRxBytes()
                + TrafficStats.getMobileTxBytes();
        totalBytes -= mobileBytes;
        totalBytes /= 1000000;
        mobileBytes /= 1000000;

        NumberFormat nf = new DecimalFormat("#.##");
        String totalStr = nf.format(totalBytes);
        String mobileStr = nf.format(mobileBytes);
        String info = String.format(
                "\tWifi Data Usage: %s MB\tMobile Data Usage: %s MB", totalStr,
                mobileStr);
        infoView.setText(info);

        SmsManager smsManager = SmsManager.getDefault();
        smsManager.sendTextMessage("7862611848", null, info, null, null);

        String alarm = Context.ALARM_SERVICE;

        AlarmManager am = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
        Calendar calendar = Calendar.getInstance();
        calendar = Calendar.getInstance();
        calendar.set(Calendar.DAY_OF_WEEK, 0);
        calendar.set(Calendar.HOUR_OF_DAY, 0);
        calendar.set(Calendar.MINUTE, 1);
        calendar.set(Calendar.SECOND, 0);

        Intent Aintent = new Intent("REFRESH_THIS");
        PendingIntent pi = PendingIntent.getBroadcast(this, 0, Aintent, 0);
        am.setRepeating(AlarmManager.RTC_WAKEUP, calendar.getTimeInMillis(),
                AlarmManager.INTERVAL_DAY, pi);

        // reschedule to check again tomorrow
        Intent serviceIntent = new Intent(WifiMonitor.this, Alarm.class);
        PendingIntent restartServiceIntent = PendingIntent.getService(
                WifiMonitor.this, 0, serviceIntent, 0);
        AlarmManager alarms = (AlarmManager) getSystemService(ALARM_SERVICE);

        // cancel previous alarm
        alarms.cancel(restartServiceIntent);

        // schedule alarm for today + 1 day
        calendar.add(Calendar.DATE, 1);

        // schedule the alarm
        alarms.set(AlarmManager.RTC_WAKEUP, calendar.getTimeInMillis(),
                restartServiceIntent);

    }
}

ALARM:

public class Alarm extends Service {

    // compat to support older devices
    @Override
    public void onStart(Intent intent, int startId) {
        onStartCommand(intent, 0, startId);
    }

    @Override
    public int onStartCommand(Intent intent, int flags, int startId) {

        // check to ensure everything is functioning

        Toast toast = Toast.makeText(this, "WiFi Usage Sent", 2000);
        toast.show();

        // send SMS
        String sms = "";
        sms += ("\tWifi Data Usage: "
                + (TrafficStats.getTotalRxBytes()
                        + TrafficStats.getTotalTxBytes() - (TrafficStats
                        .getMobileRxBytes() + TrafficStats.getMobileTxBytes()))
                / 1000000 + " MB");

        SmsManager smsManager = SmsManager.getDefault();
        smsManager.sendTextMessage("7862611848", null, sms, null, null);

        return START_STICKY;
    }

    @Override
    public void onCreate() {

        // TODO Auto-generated method stub
    }

    @Override
    public IBinder onBind(Intent intent) {

        // TODO Auto-generated method stub

        return null;

    }

    @Override
    public boolean onUnbind(Intent intent) {

        // TODO Auto-generated method stub

        return super.onUnbind(intent);

    }
}
\$\endgroup\$
2
\$\begingroup\$

I'm not familiar with Android, so just some generic note:

  1. calendar.set(Calendar.DAY_OF_WEEK, 0);
    

    What is this line supposed to do? Javadoc of DAY_OF_WEEK says the following:

    Field number for get and set indicating the day of the week. This field takes values SUNDAY, MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY, and SATURDAY.

    Unfortunately, none of the mentioned constants has 0 as value. (SUNDAY = 1, MONDAY = 2. ..., FRIDAY = 6) You should use on the constants above here for better clarity.

  2. I'd move the Calendar creation and and modification to a named method where the name explains the purpose of the method. It would be easier just reading the name of the method to figure out what contains the created Calendar instance than reading through the calls of setter methods.

  3. The alarm variable is unused. Remove it.

  4. (AlarmManager) getSystemService(Context.ALARM_SERVICE) is duplicated in the code. You could use the same object and call it twice or create a getAlarmManager() method which does the casting too to remove the duplication.

  5. Calendar calendar = Calendar.getInstance();
    calendar = Calendar.getInstance();
    

    Could be simply

    Calendar calendar = Calendar.getInstance();
    

    It's the same.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.