Split onBind() of AlarmViewHolder into multiple discrete method calls

app/src/main/java/com/philliphsu/clock2/BaseViewHolder.java

@@ -41,6 +41,10 @@ public abstract class BaseViewHolder<T> extends RecyclerView.ViewHolder implemen
         return mContext;
     }
 
+    public final T getItem() {
+        return mItem;
+    }
+
     @Override
     public final void onClick(View v) {
         if (mListener != null) {

app/src/main/java/com/philliphsu/clock2/UpcomingAlarmReceiver.java

@@ -8,7 +8,7 @@ import android.content.Context;
 import android.content.Intent;
 import android.support.v4.app.NotificationCompat;
 
-import com.philliphsu.clock2.editalarm.AlarmUtils;
+import com.philliphsu.clock2.util.AlarmUtils;
 import com.philliphsu.clock2.model.AlarmsRepository;
 
 import static android.app.PendingIntent.FLAG_ONE_SHOT;

app/src/main/java/com/philliphsu/clock2/alarms/AlarmViewHolder.java

@@ -2,11 +2,13 @@ package com.philliphsu.clock2.alarms;
 
 import android.content.SharedPreferences;
 import android.preference.PreferenceManager;
+import android.support.annotation.NonNull;
 import android.support.v7.widget.SwitchCompat;
 import android.text.SpannableString;
 import android.text.Spanned;
 import android.text.format.DateFormat;
 import android.text.style.RelativeSizeSpan;
+import android.view.View;
 import android.view.ViewGroup;
 import android.widget.Button;
 import android.widget.TextView;
@@ -16,14 +18,17 @@ import com.philliphsu.clock2.BaseViewHolder;
 import com.philliphsu.clock2.DaysOfWeek;
 import com.philliphsu.clock2.OnListItemInteractionListener;
 import com.philliphsu.clock2.R;
+import com.philliphsu.clock2.util.AlarmUtils;
 
 import java.util.Date;
 
 import butterknife.Bind;
+import butterknife.OnClick;
 
 import static android.view.View.GONE;
 import static android.view.View.VISIBLE;
 import static com.philliphsu.clock2.DaysOfWeek.NUM_DAYS;
+import static com.philliphsu.clock2.util.DateFormatUtils.formatTime;
 
 /**
  * Created by Phillip Hsu on 5/31/2016.
@@ -45,7 +50,62 @@ public class AlarmViewHolder extends BaseViewHolder<Alarm> {
     @Override
     public void onBind(Alarm alarm) {
         super.onBind(alarm);
-        String time = DateFormat.getTimeFormat(getContext()).format(new Date(alarm.ringsAt()));
+        bindTime(new Date(alarm.ringsAt()));
+        bindSwitch(alarm.isEnabled());
+        bindCountdown(alarm.isEnabled(), alarm.ringsIn());
+
+        // TODO: shared prefs
+        SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getContext());
+        // how many hours before alarm is considered upcoming
+        /*int hoursBeforeUpcoming = Integer.parseInt(prefs.getString(
+                    mContext.getString(-1TODO:R.string.key_notify_me_of_upcoming_alarms),
+                    "2"));*/
+        boolean visible = alarm.isEnabled() && (alarm.ringsWithinHours(2) || alarm.isSnoozed());
+        String buttonText = alarm.isSnoozed()
+                ? getContext().getString(R.string.title_snoozing_until, formatTime(getContext(), alarm.snoozingUntil()))
+                : getContext().getString(R.string.dismiss_now);
+        // TODO: Register dynamic broadcast receiver in this class to listen for
+        // when this alarm crosses the upcoming threshold, so we can show this button.
+        bindDismissButton(visible, buttonText);
+
+        // Should also be visible even if alarm has no label so mCountdown is properly positioned next
+        // to mLabel. That is, mCountdown's layout position is dependent on mLabel being present.
+        boolean labelVisible = alarm.label().length() > 0 || mCountdown.getVisibility() == VISIBLE;
+        bindLabel(labelVisible, alarm.label());
+
+        int num = alarm.numRecurringDays();
+        String text;
+        if (num == NUM_DAYS) {
+            text = getContext().getString(R.string.every_day);
+        } else if (num == 0) {
+            text = "";
+        } else {
+            StringBuilder sb = new StringBuilder();
+            for (int i = 0 /* Ordinal days*/; i < NUM_DAYS; i++) {
+                // What day is at this position in the week?
+                int weekDay = DaysOfWeek.getInstance(getContext()).weekDayAt(i);
+                if (alarm.isRecurring(weekDay)) {
+                    sb.append(DaysOfWeek.getLabel(weekDay)).append(", ");
+                }
+            }
+            // Cut off the last comma and space
+            sb.delete(sb.length() - 2, sb.length());
+            text = sb.toString();
+        }
+        bindDays(num > 0, text);
+    }
+
+    @OnClick(R.id.dismiss)
+    void onClick() {
+        AlarmUtils.cancelAlarm(getContext(), getItem());
+        bindDismissButton(false, ""); // Will be set to correct text the next time we bind.
+        // TODO: Check if alarm has no recurrence, then turn it off.
+    }
+
+    // TODO: Break onBind() into smaller helper method calls. Then, you can update certain
+    // pieces of the VH on demand without having to rebind the whole thing.
+    private void bindTime(Date date) {
+        String time = DateFormat.getTimeFormat(getContext()).format(date);
         if (DateFormat.is24HourFormat(getContext())) {
             mTime.setText(time);
         } else {
@@ -54,62 +114,39 @@ public class AlarmViewHolder extends BaseViewHolder<Alarm> {
             s.setSpan(AMPM_SIZE_SPAN, time.indexOf(" "), time.length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
             mTime.setText(s, TextView.BufferType.SPANNABLE);
         }
+    }
 
-        if (alarm.isEnabled()) {
+    private void bindSwitch(boolean enabled) {
-            mSwitch.setChecked(true);
+        mSwitch.setChecked(enabled);
-            //TODO:mCountdown.showAsText(alarm.ringsIn());
+    }
+
+    private void bindCountdown(boolean enabled, long remainingTime) {
+        if (enabled) {
+            //TODO:mCountdown.showAsText(remainingTime);
+            //TODO:mCountdown.getTickHandler().startTicking(true)
             mCountdown.setVisibility(VISIBLE);
-            //todo:mCountdown.getTickHandler().startTicking(true)
-            SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getContext());
-            // how many hours before alarm is considered upcoming
-            // TODO: shared prefs
-            /*int hoursBeforeUpcoming = Integer.parseInt(prefs.getString(
-                    mContext.getString(-1TODO:R.string.key_notify_me_of_upcoming_alarms),
-                    "2"));*/
-            if (alarm.ringsWithinHours(2) || alarm.isSnoozed()) {
-                // TODO: Register dynamic broadcast receiver in this class to listen for
-                // when this alarm crosses the upcoming threshold, so we can show this button.
-                mDismissButton.setVisibility(VISIBLE);
-            } else {
-                mDismissButton.setVisibility(GONE);
-            }
         } else {
-            mSwitch.setChecked(false);
-            mCountdown.setVisibility(GONE);
             //TODO:mCountdown.getTickHandler().stopTicking();
-            mDismissButton.setVisibility(GONE);
+            mCountdown.setVisibility(GONE);
-        }
-
-        mLabel.setText(alarm.label());
-        if (mLabel.length() == 0 && mCountdown.getVisibility() != VISIBLE) {
-            mLabel.setVisibility(GONE);
-        } else {
-            // needed for proper positioning of mCountdown
-            mLabel.setVisibility(VISIBLE);
-        }
-
-        int numRecurringDays = alarm.numRecurringDays();
-        if (numRecurringDays > 0) {
-            String text;
-            if (numRecurringDays == NUM_DAYS) {
-                text = getContext().getString(R.string.every_day);
-            } else {
-                StringBuilder sb = new StringBuilder();
-                for (int i = 0 /* Ordinal days*/; i < NUM_DAYS; i++) {
-                    // What day is at this position in the week?
-                    int weekDay = DaysOfWeek.getInstance(getContext()).weekDayAt(i);
-                    if (alarm.isRecurring(weekDay)) {
-                        sb.append(DaysOfWeek.getLabel(weekDay)).append(", ");
-                    }
-                }
-                // Cut off the last comma and space
-                sb.delete(sb.length() - 2, sb.length());
-                text = sb.toString();
-            }
-            mDays.setText(text);
-            mDays.setVisibility(VISIBLE);
-        } else {
-            mDays.setVisibility(GONE);
         }
     }
+
+    private void bindDismissButton(boolean visible, String buttonText) {
+        setVisibility(mDismissButton, visible);
+        mDismissButton.setText(buttonText);
+    }
+
+    private void bindLabel(boolean visible, String label) {
+        setVisibility(mLabel, visible);
+        mLabel.setText(label);
+    }
+
+    private void bindDays(boolean visible, String text) {
+        setVisibility(mDays, visible);
+        mDays.setText(text);
+    }
+
+    private void setVisibility(@NonNull View view, boolean visible) {
+        view.setVisibility(visible ? VISIBLE : GONE);
+    }
 }

app/src/main/java/com/philliphsu/clock2/editalarm/AlarmItemContract.java

@@ -2,6 +2,14 @@ package com.philliphsu.clock2.editalarm;
 
 /**
  * Created by Phillip Hsu on 6/2/2016.
+ *
+ * TODO: Consider NOT extending from AlarmContract. Instead, define the methods
+ * specific to this interface. Make a base implementation class of the base
+ * AlarmContract. When you create an implementation class of the more specific
+ * interface, all you need to do is implement the more specific interface.
+ * The base impl class will already implement the base interface methods.
+ * That way, each concrete interface impl doesn't need to implement the
+ * base interface methods again and again.
  */
 public interface AlarmItemContract {
 

app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmActivity.java

@@ -26,6 +26,7 @@ import com.philliphsu.clock2.BaseActivity;
 import com.philliphsu.clock2.DaysOfWeek;
 import com.philliphsu.clock2.R;
 import com.philliphsu.clock2.model.AlarmsRepository;
+import com.philliphsu.clock2.util.AlarmUtils;
 import com.philliphsu.clock2.util.DurationUtils;
 
 import java.util.Date;

app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmContract.java

@@ -2,6 +2,14 @@ package com.philliphsu.clock2.editalarm;
 
 /**
  * Created by Phillip Hsu on 6/2/2016.
+ *
+ * TODO: Consider NOT extending from AlarmContract. Instead, define the methods
+ * specific to this interface. Make a base implementation class of the base
+ * AlarmContract. When you create an implementation class of the more specific
+ * interface, all you need to do is implement the more specific interface.
+ * The base impl class will already implement the base interface methods.
+ * That way, each concrete interface impl doesn't need to implement the
+ * base interface methods again and again.
  */
 public interface EditAlarmContract {
 

app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java

@@ -15,7 +15,7 @@ import android.widget.Toast;
 
 import com.philliphsu.clock2.Alarm;
 import com.philliphsu.clock2.R;
-import com.philliphsu.clock2.editalarm.AlarmUtils;
+import com.philliphsu.clock2.util.AlarmUtils;
 import com.philliphsu.clock2.model.AlarmsRepository;
 
 import static com.philliphsu.clock2.util.DateFormatUtils.formatTime;

app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java

@@ -19,7 +19,7 @@ import android.util.Log;
 
 import com.philliphsu.clock2.Alarm;
 import com.philliphsu.clock2.R;
-import com.philliphsu.clock2.editalarm.AlarmUtils;
+import com.philliphsu.clock2.util.AlarmUtils;
 import com.philliphsu.clock2.model.AlarmsRepository;
 
 import static com.philliphsu.clock2.util.DateFormatUtils.formatTime;
@@ -77,11 +77,13 @@ public class RingtoneService extends Service { // TODO: abstract this, make subc
     @Override
     public int onStartCommand(Intent intent, int flags, int startId) {
         // Although this is a bound service, we override this method because this class is reused for
-        // handling the notification actions for the presently ringing alarm. From the docs of Context#startService():
+        // handling the notification actions for the presently ringing alarm.
+        // Although the docs of Context#startService() says this:
         // "Using startService() overrides the default service lifetime that is managed by
         // bindService(Intent, ServiceConnection, int): it requires the service to remain running until
-        // stopService(Intent)* is called, regardless of whether any clients are connected to it."
+        // stopService(Intent) [or stopSelf()] is called, regardless of whether any clients are connected to it."
-        // * Would stopSelf() also work?
+        // I have found the regardless part does not apply here. You MUST also unbind any clients from this service
+        // at the same time you stop this service!
 
         if (ACTION_SNOOZE.equals(intent.getAction())) {
             long id = intent.getLongExtra(EXTRA_ITEM_ID, -1);
@@ -92,7 +94,7 @@ public class RingtoneService extends Service { // TODO: abstract this, make subc
         }
         stopSelf(startId);
         if (mRingtoneCallback != null) {
-            mRingtoneCallback.onServiceFinish();
+            mRingtoneCallback.onServiceFinish(); // tell client to unbind from this service
         }
 
         return START_NOT_STICKY; // If killed while started, don't recreate. Should be sufficient.

app/src/main/java/com/philliphsu/clock2/util/AlarmUtils.java

@@ -1,4 +1,4 @@
-package com.philliphsu.clock2.editalarm;
+package com.philliphsu.clock2.util;
 
 import android.app.AlarmManager;
 import android.app.PendingIntent;

app/src/main/java/com/philliphsu/clock2/util/DurationUtils.java

@@ -13,6 +13,8 @@ import java.util.concurrent.TimeUnit;
  */
 public class DurationUtils {
 
+    /** Return a string representing the duration, formatted in hours and minutes.
+     * TODO: Need to adapt this to represent all time fields eventually */
     public static String toString(Context context, long millis, boolean abbreviate) {
         long[] fields = breakdown(millis);
         long numHours = fields[0];
@@ -20,6 +22,11 @@ public class DurationUtils {
         long numSecs = fields[2]; // only considered for rounding of minutes
         if (numSecs >= 31) {
             numMins++;
+            numSecs = 0; // Not totally necessary since it won't be considered any more
+            if (numMins == 60) {
+                numHours++;
+                numMins = 0;
+            }
         }
 
         @StringRes int res;

app/src/main/res/values/strings.xml

@@ -37,13 +37,13 @@
     <string name="abbrev_minutes">%2$dm</string>
     <string name="abbrev_less_than_one_minute">&lt;1m</string>
     <!-- ======================================================================================= -->
-    
+
     <!-- ==================================== NOTIFICATIONS ==================================== -->
     <string name="upcoming_alarm">Upcoming alarm</string>
     <string name="alarm">Alarm</string>
     <string name="missed_alarm">Missed alarm</string>
     <!-- ======================================================================================= -->
-    
+
     <!-- ==================================== MAIN ACTIVITY ==================================== -->
     <string name="snackbar_item_deleted">%1$s deleted</string>
     <string name="snackbar_undo_item_deleted">Undo</string>