Time manipulation for notification reminders

Question

I'm new to programming, Stack Overflow and Code Review. I was encouraged to post this method here based on a question I posted on StackOverflow here. Let me know if you have tips for improving this post, and I will try to implement them asap.

I'm hoping for a general review of my code. I realize the formatting is going to be less than perfect and my code isn't a gleaming example of best programming practices (i.e., write small functions, use MVC, etc.). While I'd certainly appreciate all comments about formatting and readability of my code, I'm much more interested whether there any of my code is likely to cause significant performance issues / crashing / etc.

A few questions:

1. Considering I'm building a small app to test an idea, I think that writing the shortest, highest performing code should not be my focus. However, the code needs to work, so any major defects causing crashing or a negative user experience are harmful. Is this line of thinking flawed?

2. Can the 80/20 rule apply to programming? In other words, what are the 20% changes to implement that would make up 80% of the possible performance increase?

3. If one were to call two methods back to back in viewWillAppear, such as…

   doSomething()
   eveningReminderTimes()
   

   … I think eveningReminderTimes() won't start executing until doSomething() has finished, correct?

4. I am emptying arrays via arrayName = [], for example. It feels like it might be better for performance to replace the current values when needed. Is there harm in emptying a small 7-index String/Int array like this vs some kind of replacement alternative?

5. I've received a few comments on Stack Overflow suggesting that you I write smaller functions. I've also heard this on a couple programming podcasts, but I haven't heard much, if any, convincing insight as to why. If one has a connecting set of functionality that must always be performed together, what is the benefit of splitting it up?

Please consider question 5 with the assumption that the programmer/developer intends to build one app only and has no plans to build future apps (intended to combat the code reuse for other app argument).

If the connected code that will always be called together is split up, there is more space separating such connected code (detriment for readability, I would think). And it introduces the possibility that the programmer may forget to call one of the connected methods (potential for a bug). If you need to separate the larger method into smaller methods later because it becomes necessary, why not wait until that later time when it becomes necessary? It seems like you may be reducing readability and increase the possibility for human programming error if you proactively split up larger functions of connected code.

Here's the code, which works in a Playground. I really appreciate any help.

import Foundation
import UIKit

class ViewController: UIViewController {

    var settingsArray = [String]()
    var sleepHoursArray = [Int]()
    var sleepMinutesArray = [Int]()
    var sleepTimeArray = [String]()

    var bedTimeMinutesArray = [Int]()
    var bedTimeHoursArray = [Int]()
    var bedTimeAMorPMArray = [String]()
    var bedTimeArray = [String]()

    var morningAlarmHoursArray = [Int]()
    var morningAlarmMinutesArray = [Int]()
    var morningAlarmAMorPMArray = [String]()

    var totalNiteChoreTimeHoursArray = [Int]()
    var totalNiteChoreTimeMinutesArray = [Int]()

    var eveningReminderTimeMinutesArray = [Int]()
    var eveningReminderTimeHoursArray = [Int]()
    var eveningReminderTimeAMorPMArray = [String]()

    var sundayEveningArray = [Int]()
    var mondayEveningArray = [Int]()
    var tuesdayEveningArray = [Int]()
    var wednesdayEveningArray = [Int]()
    var thursdayEveningArray = [Int]()
    var fridayEveningArray = [Int]()
    var saturdayEveningArray = [Int]()

    override func viewDidLoad() {
        super.viewDidLoad()

    }

    override func viewWillAppear(animated: Bool) {

        settingsArray = ["7h 10m", "7h 10m", "7h 10m", "7h 10m", "7h 10m", "7h 10m", "7h 10m"]
        morningAlarmMinutesArray = [51, 52, 53, 54, 55, 56, 57]
        morningAlarmHoursArray = [5, 5, 5, 5, 5, 5, 5]
        sundayEveningArray = [1, 10]
        mondayEveningArray = [1, 10]
        tuesdayEveningArray = [1, 10]
        wednesdayEveningArray = [1, 10]
        thursdayEveningArray = [1, 10]
        fridayEveningArray = [1, 10]
        saturdayEveningArray = [1, 10]

        eveningReminderTimes()

    }

    func eveningReminderTimes() {

        //Build the sleep arrays
        sleepHoursArray = []
        sleepMinutesArray = []

        for i in 0...6 {
            let sleepHoursAndMinutes = settingsArray[i]

            var separateBySpace = sleepHoursAndMinutes.componentsSeparatedByString(" ")
            let hours = separateBySpace[0]
            let hours2 = String(hours.characters.dropLast())

            let minutes = separateBySpace[1]
            let minutes2 = String(minutes.characters.dropLast())

            let x = Int(hours2)!
            let y = Int(minutes2)!

            sleepHoursArray.append(x)
            sleepMinutesArray.append(y)
        }

        //Build the bed time array

        var morningAlarmHourLessSleepHour = 0

        bedTimeHoursArray = []

        for hours in 0...6 {
            var morningHours = hours + 1
            if morningHours == 7 {
                morningHours = 0
            }
            morningAlarmHourLessSleepHour = Int(morningAlarmHoursArray[morningHours]) - Int(sleepHoursArray[hours])

         //TO CODE REVIEW: While writing this question, I realized the "Int(...)" isn't needed above.  Could this cause crashing for some users?

            bedTimeHoursArray.append(morningAlarmHourLessSleepHour)
        }

        var morningAlarmMinutesLessSleepMinutes = 0
        bedTimeMinutesArray = []
        bedTimeAMorPMArray = []

        for minutes in 0...6 {

            var bedTimeAMorPM = "AM"

            //needed since you use the next day's morning alarm to calculate the previous day's bedtime
            var morningMinutes = minutes + 1
            if morningMinutes == 7 {
                morningMinutes = 0
            }

            morningAlarmMinutesLessSleepMinutes = Int(morningAlarmMinutesArray[morningMinutes]) - Int(sleepMinutesArray[minutes])

        //TO CODE REVIEW: While writing this question, I realized the "Int(...)" isn't needed above.  Could this cause crashing for some users?

            if morningAlarmMinutesLessSleepMinutes < 0 {
                morningAlarmMinutesLessSleepMinutes = morningAlarmMinutesLessSleepMinutes + 60
                bedTimeHoursArray[minutes] = bedTimeHoursArray[minutes] - 1
            }

            switch bedTimeHoursArray[minutes] {
            case 0:
                bedTimeHoursArray[minutes] = bedTimeHoursArray[minutes] + 12
                break
            case -11...(-1):
                bedTimeHoursArray[minutes] = bedTimeHoursArray[minutes] + 12
                bedTimeAMorPM = "PM"
                break
            case -24...(-12):
                bedTimeHoursArray[minutes] = bedTimeHoursArray[minutes] + 24
                bedTimeAMorPM = "AM"
                break
            default:
                break
            }
            bedTimeMinutesArray.append(morningAlarmMinutesLessSleepMinutes)
            bedTimeAMorPMArray.append(bedTimeAMorPM)
        }

        //Build the Total Evening Chore Hours and Minutes Arrays

        totalNiteChoreTimeHoursArray = []
        totalNiteChoreTimeMinutesArray = []

        buildTotalEveningHoursAndMinuteArrays(sundayEveningArray)
        buildTotalEveningHoursAndMinuteArrays(mondayEveningArray)
        buildTotalEveningHoursAndMinuteArrays(tuesdayEveningArray)
        buildTotalEveningHoursAndMinuteArrays(wednesdayEveningArray)
        buildTotalEveningHoursAndMinuteArrays(thursdayEveningArray)
        buildTotalEveningHoursAndMinuteArrays(fridayEveningArray)
        buildTotalEveningHoursAndMinuteArrays(saturdayEveningArray)

        //Build the evening reminder time array

        var bedTimeHourLessEveningChoreHour = 0

        eveningReminderTimeHoursArray = []

        for hours in 0...6 {
            bedTimeHourLessEveningChoreHour = Int(bedTimeHoursArray[hours]) - Int(totalNiteChoreTimeHoursArray[hours])

            //TO CODE REVIEW: While writing this question, I realized the "Int(...)" isn't needed above.  Could this cause crashing for some users?

            eveningReminderTimeHoursArray.append(bedTimeHourLessEveningChoreHour)
        }

        var bedTimeMinutesLessEveningChoreMinutes = 0
        eveningReminderTimeMinutesArray = []
        eveningReminderTimeAMorPMArray = []

        for minutes in 0...6 {
            var eveningReminderTimeAMorPM = bedTimeAMorPMArray[minutes]

            bedTimeMinutesLessEveningChoreMinutes = Int(bedTimeMinutesArray[minutes]) - Int(totalNiteChoreTimeMinutesArray[minutes])

            if bedTimeMinutesLessEveningChoreMinutes < 0 {
                bedTimeMinutesLessEveningChoreMinutes = bedTimeMinutesLessEveningChoreMinutes + 60
                eveningReminderTimeHoursArray[minutes] = eveningReminderTimeHoursArray[minutes] - 1
            }

            switch eveningReminderTimeHoursArray[minutes] {
            case 1...12:
                if bedTimeAMorPMArray[minutes] == "AM" {
                    if bedTimeHoursArray[minutes] < eveningReminderTimeHoursArray[minutes]{
                        eveningReminderTimeAMorPM = "PM"
                    }
                    if bedTimeHoursArray[minutes] == 12 && eveningReminderTimeHoursArray[minutes] < 12 {
                        eveningReminderTimeAMorPM = "PM"
                    }
                }
                break

            case 0:
                eveningReminderTimeHoursArray[minutes] = eveningReminderTimeHoursArray[minutes] + 12
                break

            case -11...(-1):
                eveningReminderTimeHoursArray[minutes] = eveningReminderTimeHoursArray[minutes] + 12

                if eveningReminderTimeAMorPM == "AM" {
                    eveningReminderTimeAMorPM = "PM"
                } else {
                    eveningReminderTimeAMorPM = "AM"
                }
                break

            case -24...(-12):
                eveningReminderTimeHoursArray[minutes] = eveningReminderTimeHoursArray[minutes] + 24
                if eveningReminderTimeAMorPM == "AM" {
                    eveningReminderTimeAMorPM = "AM"
                } else {
                    eveningReminderTimeAMorPM = "PM"
                }
                break

            default:
                break
            }
            eveningReminderTimeMinutesArray.append(bedTimeMinutesLessEveningChoreMinutes)
            eveningReminderTimeAMorPMArray.append(eveningReminderTimeAMorPM)
        }

        print(eveningReminderTimeHoursArray)
        print(eveningReminderTimeMinutesArray)
        print(eveningReminderTimeAMorPMArray)

    }

    func buildTotalEveningHoursAndMinuteArrays(eveningArray: Array<Int>) {

        var totalNiteChoreTimeSum = 0
        var totalNiteChoreTimeHours = 0
        var totalNiteChoreTimeMinutes = 0

        for itemTime in 1...eveningArray.count {

            var totalNiteChoreTime = 0
            let index = itemTime - 1

            totalNiteChoreTime = eveningArray[index]
            totalNiteChoreTimeSum += totalNiteChoreTime
            totalNiteChoreTimeHours = totalNiteChoreTimeSum / 60
            totalNiteChoreTimeMinutes = totalNiteChoreTimeSum % 60

        }

        totalNiteChoreTimeHoursArray.append(totalNiteChoreTimeHours)
        totalNiteChoreTimeMinutesArray.append(totalNiteChoreTimeMinutes)
    }

}

Answer 1

So for now, this review will primary address your listed questions rather than actually taking a look at your code.

---

Considering I'm building a small app to test an idea, I think that writing the shortest, highest performing code should not be my focus. However, the code needs to work, so any major defects causing crashing or a negative user experience are harmful. Is this line of thinking flawed?

No, this line of thinking is not flawed. You should define a set of criteria by which you will decide whether or not your app works good enough for production. We do want to minimize things like crashing and unwanted behavior (bugs), but we don't generally prioritize performance (neither time or memory) out the gate. Even as a professional iOS developer, I only worry myself with these things when something has been measured and proven to be a problem from a performance stand point. And even then, it's usually not something addressed on the first iteration.

In the first iteration, we get a working code base. But what's important in the first iteration is to get a relatively clean code base. Importantly, something that is easy to maintain, and unit tests are a major bonus. When we come to refactor this to improve performance, our unit tests will assure we don't have regressions. And we can write unit tests that measure the time execution and ensure that's improving rather than getting worse.

And unit testing is just one of the many reasons why we want to break our code down into smaller methods and chunks... but I'm not ready to drill that in quite yet. We will revisit it.

But with all this said, we shouldn't ignore obvious performance concerns. So you may very well get some reviews that suggest a different way to do something that is better from a performance point of view. Again, while it probably shouldn't be your main focus, you certainly shouldn't ignore the reviews.

---

Can the 80/20 rule apply to programming? In other words, what are the 20% changes to implement that would make up 80% of the possible performance increase?

I've never heard of this rule. You should certainly prefer to work smarter rather than harder. But before you focus on performance, you should be sure to identify the bottlenecks. There exist tools (like the Instruments app) which help you measure and identify the parts of your app that are least performant. Start with the bottlenecks.

---

If one were to call two methods back to back in viewWillAppear, such as…

doSomething() 
eveningReminderTimes()

… I think eveningReminderTimes() won't start executing until doSomething() has finished, correct?

Correct. If you're not dispatching anything asynchronously, then your code runs synchronously. If you're ever in doubt, you can use breakpoints in the debugger or logging statements to help you see the execution flow.

---

I am emptying arrays via arrayName = [], for example. It feels like it might be better for performance to replace the current values when needed. Is there harm in emptying a small 7-index String/Int array like this vs some kind of replacement alternative?

Have you measured performance? Going back to my previous comments, we shouldn't overly concern ourselves with performance until we measure it and determine it's not good enough per our acceptance criteria. And it's important to note that "as fast as possible" is generally not considered acceptable criteria in a professional setting, where "less than 10 milliseconds" is. And with criteria like that, it doesn't matter if it runs in 1 millisecond, 5 milliseconds, or 9.999 milliseconds. If it's less than 10 milliseconds, it's acceptable. You write tests to prove it is meeting that criteria, and you move on to the next acceptance criteria and stop worrying about how fast this particular thing is.

With that said, when this section of code is reviewed, you very well may get some alternative suggestions here. There may be things that are not only better performance-wise, but also better from a readability stand point. There is a difference between an experienced developer who know by experience which way is better so with minimal time investment make performance improvements and an inexperienced developer spending more time than necessary trying to improve the performance of a piece of code that is already running within the acceptable criteria. But importantly, the suggestion to change this is probably only coincidentally more performant, and the real reason for the suggestion is increased readability.

Readability is very important. You should pretty much always code for readability first, and only write things at the expense of readability when there's no good way to write code that's clearly readable and still get the required performance. And when you've made this sacrifice to readability, the code that's hard to read should be tucked away inside a well-named method with comments to explain the complicated stuff going on inside the method.

---

I've received a few comments on Stack Overflow suggesting that you I write smaller functions. I've also heard this on a couple programming podcasts, but I haven't heard much, if any, convincing insight as to why. If one has a connecting set of functionality that must always be performed together, what is the benefit of splitting it up?

There are plenty of good reasons to break up logic into smaller, digestible chunks.

For some extra reading, I'll go ahead and recommend these articles:

• Writing Readable Code
• How do you eat an elephant?
• Unit Tests Don't Prevent Bugs

So, there are several reasons that go into making smaller methods advantageous.

One big reason is still going to be code reuse, even in light of this comment:

Please consider question 5 with the assumption that the programmer/developer intends to build one app only and has no plans to build future apps (intended to combat the code reuse for other app argument).

The fact of the matter is, well-written applications reuse code within themselves. And when possible, instead of writing your own code, you should pull in libraries that have already implemented the functionality you need instead of writing it all from scratch yourself.

Your buildTotalEveningHoursAndMinuteArrays is a perfectly good example of code reuse within the app itself. You're calling this method at least seven times, by my count. But I'd argue, for example, that the code within the loop in this method could also be extracted into its own method.

But still, buildTotalEveningHoursAndMinuteArrays is a good example of a somewhat decently named method that does almost just one thing. And if we rewrote it to take an input and give an output instead of assigning to an instance variable, this would be a very clear and easy candidate for a unit test (it can still be unit tested as is, but it could be better).

And that leads me into the second point about writing smaller methods: unit tests.

The larger and more complicated your methods are, the harder they are to unit test completely. Methods should do one thing. They should be considered a single unit of code, and unit tests should assure they do that one thing well. Unit tests guarantee that no matter what refactoring happens to the method itself, you haven't broken at least the functionality that your unit tests expect. And the more thorough your unit tests, the better. They'll prevent regressions when you're refactoring to improve readability, performance, or eliminate crashes.

And finally, smaller methods drastically help the long term maintainability of the application. Long term maintainability (anything past 3 months) becomes difficult when we have methods like doFiftyThings() instead of fifty methods called doMyOneThingThatIDoWell(). When you have numerous small methods with good names, it helps maintainers encapsulate a handful of lines of codes under a single name. A methods name should be a plain English description of everything that the method does.

So eveningReminderTimes is a bad method name on several levels. First of all, it describes a thing, not an action, so I'd expect it to either be a variable I can access or a method with a return value. But it's not either of these. It's calculating the evening reminder times and then assigning them to an instance variable.

Instead, what makes this code a lot more readable and a lot easier to clearly see what's going on, I'd expect to see code that looks something more like this:

weeklyEveningReminderTimes = calculateEveningRemindersForWeek()

Where that method is implemented as something like this...

func calculateEveningRemindersForWeek(containingDate date: NSDate = NSDate()) -> [Reminder] {
    // determine the right week based on the passed in date
    // method has a default argument of the current time, so current week
    // we build the seven days into an array called current week

    return currentWeek.map { calculateRemindersForDay($0) }
}

Where calculateRemindersForDay is some logic that calculates the reminders just for the passed in day and returns those values.

Answer 2

Now then... let's look at some code...

---

Instance Variables

var settingsArray = [String]()
var sleepHoursArray = [Int]()
var sleepMinutesArray = [Int]()
var sleepTimeArray = [String]()

var bedTimeMinutesArray = [Int]()
var bedTimeHoursArray = [Int]()
var bedTimeAMorPMArray = [String]()
var bedTimeArray = [String]()

var morningAlarmHoursArray = [Int]()
var morningAlarmMinutesArray = [Int]()
var morningAlarmAMorPMArray = [String]()

var totalNiteChoreTimeHoursArray = [Int]()
var totalNiteChoreTimeMinutesArray = [Int]()

var eveningReminderTimeMinutesArray = [Int]()
var eveningReminderTimeHoursArray = [Int]()
var eveningReminderTimeAMorPMArray = [String]()

var sundayEveningArray = [Int]()
var mondayEveningArray = [Int]()
var tuesdayEveningArray = [Int]()
var wednesdayEveningArray = [Int]()
var thursdayEveningArray = [Int]()
var fridayEveningArray = [Int]()
var saturdayEveningArray = [Int]()

This is a very naïve approach to the problem. There are several problems with just this section alone. The over arching problem is you just have way too many variables. A clear indication that this file has become a dumping ground for code and it's trying to do way too much.

Part of the reason we have two many variables here is because we're not using any good types. We're sticking to strings and integers, but it's clear based on the names of these arrays that we could use something better.

And just as a note... we don't need to postpend Array to everything. Just use a good variable name. Usually, a plural is indicator enough that we're dealing with a collection of things.

So... I see several array groupings for which the grouping can be replaced by a single array.

Consider these variables:

var bedTimes = [NSDate]()
var morningAlarms = [NSDate]()
var eveningReminders = [NSDate]()

var sleepTimes = [NSTimeInterval]()
var nightChoreTimes = [NSTimeInterval]()

This maybe isn't perfect, and something like those days of the week arrays, I haven't reviewed the section of code that uses those yet, so I'm not sure how things are used, but you see how simply using better types and better names, we really clean things up quickly?

---

Life Cycle Events

If you're not adding any code in an override besides simply calling super, you can simply delete the method.

override func viewDidLoad() {
    super.viewDidLoad()

}

But if you are overriding, you should generally be sure to call super.

override func viewWillAppear(animated: Bool) {

    settingsArray = ["7h 10m", "7h 10m", "7h 10m", "7h 10m", "7h 10m", "7h 10m", "7h 10m"]
    morningAlarmMinutesArray = [51, 52, 53, 54, 55, 56, 57]
    morningAlarmHoursArray = [5, 5, 5, 5, 5, 5, 5]
    sundayEveningArray = [1, 10]
    mondayEveningArray = [1, 10]
    tuesdayEveningArray = [1, 10]
    wednesdayEveningArray = [1, 10]
    thursdayEveningArray = [1, 10]
    fridayEveningArray = [1, 10]
    saturdayEveningArray = [1, 10]

    eveningReminderTimes()

}

And life cycle event methods shouldn't be filled with code like this. These values could be set as the defaults for the arrays when you declare them. But even if not, you should move these down into a function like setInitialValues, and call that from viewWillAppear.

---

This should be one line of code...

func buildTotalEveningHoursAndMinuteArrays(eveningArray: Array<Int>) {

    var totalNiteChoreTimeSum = 0
    var totalNiteChoreTimeHours = 0
    var totalNiteChoreTimeMinutes = 0

    for itemTime in 1...eveningArray.count {

        var totalNiteChoreTime = 0
        let index = itemTime - 1

        totalNiteChoreTime = eveningArray[index]
        totalNiteChoreTimeSum += totalNiteChoreTime
        totalNiteChoreTimeHours = totalNiteChoreTimeSum / 60
        totalNiteChoreTimeMinutes = totalNiteChoreTimeSum % 60

    }

    totalNiteChoreTimeHoursArray.append(totalNiteChoreTimeHours)
    totalNiteChoreTimeMinutesArray.append(totalNiteChoreTimeMinutes)
}

So, as mentioned in my other answer, these methods should generally accept a value and return a value rather than setting an instance method. And if they can do that, then they can be moved completely out of this view controller class and placed somewhere more appropriate, doing some utility functions.

But there's some weirdness going on.

First of all, these two lines:

totalNiteChoreTimeHours = totalNiteChoreTimeSum / 60
totalNiteChoreTimeMinutes = totalNiteChoreTimeSum % 60

We are redundantly reassigning to these properties on every iteration of the loop. Why? Why not just calculate these values at the end of the loop? Or better yet, why don't we cut the amount of space we're storing in half and just store the amount of minutes. We only need to calculate hours later and presumably only for display to the user. How we display information to the user should very rarely have much of an impact on how we actually model that information in our code. Calculating minutes & hours out of minutes is relatively inexpensive, and is probably ultimately less expensive then the time we spend storing and allocating memory for the second array.

Then there's this:

for itemTime in 1...eveningArray.count {

    var totalNiteChoreTime = 0
    let index = itemTime - 1

The totalNiteChoreTime variable is entirely unnecessary... but I'm not that concerned about that one actually. What's completely unclear to me is why we're going through the array backward (and there's a better way to do it anyway). We just need a sum of all of the values in the array, and last I checked, 1 + 2 + 3 + 4 adds up the same as 4 + 3 + 2 + 1. There's going to be no difference whether we go through the array forward or backward as far as I can tell.

So, ultimately, what's this method actually doing, big picture? It looks like it's taking an array of integers which represent time, summing them, and appending this summed time as hours & minutes to other arrays.

So, if we accept my proposition that storing the value as simply total minutes and calculating that into minutes/hours as needed is better, and my proposition that rather than directly appending to the arrays, we should write a function with an input and output, we're moving toward a pretty simply solution. And we like simple.

Now, the thing is, I don't like using Int here, because that doesn't tell us anything. There is NSTimeInterval, which is a good way in iOS apps to represent time. It's documented as representing seconds, but it's also a floating point number. What I do like about NSTimeInterval is that we know exactly what it represents. Maybe you don't want seconds though. If you're not okay with using seconds, or if you're not okay with a floating point number, I recommend using your own typealias, as such:

typealias Minute = Int

This makes our code more self documented. Of course this method expects an array that represents measure of time in minutes and returns a measure of time in minutes... that's what types it takes in and pass out.

So we end up with a signature about like this:

typealias Minute = Int

func calculateTotalMinutes(minutes: [Minute]) -> Minute

But... I'll make the case that maybe we don't even need this function at all... because this is one line of code in Swift.

func calculateTotalMinutes(minutes: [Minute]) -> Minute {
    return minutes.reduce(0, combine: +)
}

So if I've got an array of minutes, and I want to know the total hours & minutes in that array, the code looks like this:

let multipleTimeAmounts: [Minute] = // ... initialize

let totalTime = multipleTimeAmounts.reduce(0, combine: +)

// then whenever you need hours or minutes:
let hours = totalTime / 60
let minutes = totalTime % 60

---

eveningReminderTimes()

I'm sure there's lots of clean up & refactoring that can happen here, and perhaps I'll come back and append to this answer or write yet another answer, but what I'd really like to see is a follow up posting in which you've applied some of the things I've pointed out in answer to this question plus invested some effort applying those things to eveningReminderTimes() as well. As it stands, this answer is plenty long enough for now.