Memory/Cpu improvement #5
Conversation
calonso
left a comment
calonso
left a comment
There was a problem hiding this comment.
Great catch realising that we don't need to reload the images everytime. It should have a CPU impact for sure. Just a minor comments in order to keep it even simpler.
Thanks, great work!
|
|
||
| - (void)updateLocations:(NSTimer *)timer { | ||
| [radar updatePoints:centerCoordinate]; | ||
| [radar updatePoints:centerCoordinate reloadData:NO]; |
There was a problem hiding this comment.
If it is never invoked with YES then, I'd remove the new argument.
There was a problem hiding this comment.
Reloaddata wont be called here, but if you want to call it manually you could call it somewhere else from superview on data changes without reallocating everything.
| } | ||
|
|
||
| // Only remove subviews if reloading the data with new points | ||
| if (reloadData) { |
There was a problem hiding this comment.
Data will never be reloaded so remove this bit. Instead I'd check somehow that if this is the first call (i.e., no dots in the dotsView), then dot views have to be created.
There was a problem hiding this comment.
The point was for compatibility if you want to invoke reloaddata from superview without reallocating the entire radarview from beginning, for example you change positions and want to change the number of points.
There was a problem hiding this comment.
If it is needed at some point then it will be added. For now I'd rather keep it the simplest. :)
| UIImageView *point; | ||
|
|
||
| // Only create new imageviews if reloaddata or initial load | ||
| if (points.count != dotsView.subviews.count) { |
There was a problem hiding this comment.
All dot views will already be created (from line 40), so we just need to point = [dotsView.subviews objectAtIndex:count]; here.
| point = [dotsView.subviews objectAtIndex:count]; | ||
| } | ||
|
|
||
| point.tintColor = [UIColor redColor]; |
There was a problem hiding this comment.
I forgot to clean that part , it was more of an indicator if you want to change the standard color of the image.
|
Hi @Sneakr how are you doing with this? Do you need any help? |
|
@calonso hey, I asked what to do to get this merged , maybe you miseed that message, this is my first pull request if you can point me in direction I can finish it of course, not sure what to do from here lol! |
|
Ok, so the way this works is that normally you go through my comments fixing them, commit and push changes again, I review and make comments and so on until we get to a point where we are both happy with the final result and we merge it. Hopfeully it won't take very long to get to this point as your changes are quite good already and my comments are just minor ones. |
|
Hey @Sneakr how are you doing? Do you need any help with this? |
|
Hey man, Sorry for the delay im not very active, This github interface really makes me dizzy, my I get brainfreeze from everything here, you are free to just copy the edits and make the improvments as you wish and merge it! |
|
That's a pity man, we were so close, just a couple of minor comments... |
2 participants
Changed bad recreation of UIImageViews on every movement frequency cycle and saves performance.