patternswiftMinor
Time manipulation for notification reminders
Viewed 0 times
reminderstimemanipulationfornotification
Problem
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:
-
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?
-
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?
-
If one were to call two methods back to back in
… I think
-
I am emptying arrays via
-
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 conside
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:
-
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?
-
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?
-
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?-
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?-
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 conside
Solution
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…
… I think
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
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
Code Snippets
doSomething()
eveningReminderTimes()weeklyEveningReminderTimes = calculateEveningRemindersForWeek()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) }
}Context
StackExchange Code Review Q#125411, answer score: 2
Revisions (0)
No revisions yet.