Memory leaks - formatting a string to display time, every second

Question

Hey guys. I have a method that is called every second that I want to use to display the time during which my application is running. Currently, the class I'm using (which I did not create) has a property called progress , which stores the total number of seconds.

I already wrote some code that takes these seconds and formats it into a readable string. I'm new to this, so forgive me if this is not the best code. I welcome any suggestions:

 // hours, minutes, and seconds are instance variables defined as integers int totalSeconds = (int)streamer.progress; hours = totalSeconds / (60 * 60); if (hours > 0) formattedTimeString = [NSString stringWithFormat:@"%d:", hours]; // WRONG minutes = (totalSeconds / 60) % 60; seconds = totalSeconds % 60; [formattedTimeString stringByAppendingFormat:@"%d:%d", minutes, seconds]; // WRONG 

Basically, I want it to display as "3:35", for example, to show 3 minutes, 35 seconds. I want to show only the hour section if it was an hour, in which case it would be “2: 3: 35”, for example (can anyone recommend a better way to format this?).

Problem I have where I actually create / set the line (lines marked with WRONG). Since this runs every second, I would easily get a leak if I continue to query for a new row object. I suppose I can solve this by issuing a foramttedTimeString at the end of the method, but is this the right way to do this? Did NSMutableString help? Is there a better way for Cocoa to do this? I already asked at #iphonedev @freenode, and they said that I would have to write this method myself, but I decided that I would ask again.

Provide context : this is an application for streaming Internet radio (I know that many already exist, but I just practice). I want to show the amount of time the stream was playing.

Sorry if this question is stupid, heh, as I said, I'm new to this.

Answer 1

I would do it something like this:

 int totalSeconds = (int)streamer.progress; hours = totalSeconds / (60 * 60); minutes = (totalSeconds / 60) % 60; seconds = totalSeconds % 60; if ( hours > 0 ) { formattedTimeString = [NSString stringWithFormat:@"%d:%02d:%02d", hours, minutes, seconds]; } else { formattedTimeString = [NSString stringWithFormat:@"%d:%02d", minutes, seconds]; } 

Now, in the end, formattedTimeString is the desired time, but you do not “own” it - you must save it or save it in the “copy” property if you want to save it.

Note that% 02d gives you two digits, the zero filled number, which is usually required for numbers several times.

To see how you do it with stringByAppendingFormat, it will look something like this:

 NSString* formattedTimeString = @""; if ( hours > 0 ) { formattedTimeString = [formattedTimeString stringByAppendingFormat:@"%d:", hours]; } formattedTimeString = [formattedTimeString stringByAppendingFormat:@"%d:%02d", minutes, seconds]; 

However, in this case, you will get a time similar to 3: 4: 05, more than the more desirable 3:04:05.

Please note that formattedTimeString is overwritten every time, but it’s OK because you don’t “own” it at any time, so you are responsible for releasing it.

Finally, to see it with a mutable string, it might look like this:

 NSMutableString* formattedTimeString = [NSMutableString string]; if ( hours > 0 ) { [formattedTimeString appendFormat:@"%d:", hours]; } [formattedTimeString appendFormat:@"%d:%02d", minutes, seconds]; 

Again, the result of the time is an undesirable 3: 4: 05, and again you don't have formattedTimeString at the end, so you need to save it or save it using the copy property to save it.

Answer 2

To recognize delta as a unit of time, you can also do something like this:

 // as part of init... self.gregorian = [[NSCalendar alloc] initWithCalendarIdentifier:NSGregorianCalendar]; // in the timer or wherever you are tracking time deltas... static NSUInteger unitFlags = NSHourCalendarUnit | NSMinuteCalendarUnit | NSSecondCalendarUnit; NSDateComponents *components = [gregorian components:unitFlags fromDate:myBaseTime toDate:[NSDate date] options:0]; 

You can then reference parts with something like this [components minute] . Remember that you will need to open the calendar in dealloc .

Answer 3

Your code looks good. You do not leak any memory, because the created string objects have a counter with a zero value and will be cleared by the system. However, if formattedTimeString is not a local variable in your function, you need to save it at the end to prevent this from happening! To do this, you must add [formattedTimeString save] to the end of your code block, and then you must add [formattedTimeString release] before replacing the string object.

Typically, functions with names containing "alloc", "copy", "create" and "new" return objects that have already been saved (which means that their save value is +1). It is your responsibility to trigger release or auto-advertising on these objects when you use them, or they simply begin to accumulate in your memory.

Functions such as "stringWithFormat:", "imageNamed:" and "arrayWithCapacity:" return objects with a zero hold value, so you can safely drop them (as you did in the code example). If you want to keep them, you must call them to make sure they are not cleaned while you use them.

All that said, I think the main problem is using stringByAppendingFormat :. Since the NSString used is not changed, this call returns a new line. You want to say:

formattedTimeString = [formattedTimeString stringByAppendingFormat: @ "% d:% d", minutes, seconds];

Alternatively, you can use NSMutableString. Since this is what you will do over and over again, I would recommend doing it. Technically, this is normal anyway.

Hope this helps! The whole save / release thing can get confused. Just remember that each object has a "keepCount", and after it reaches zero, it is not reported what happens to the object or its data.

Answer 4

Hi guys, I appreciate the feedback.

I finished this and it works, but I would like to know if you have problems with it:

 int totalSeconds = (int)streamer.progress; [formattedTimeString setString:@""]; hours = totalSeconds / (60 * 60); if (hours > 0) [formattedTimeString appendFormat:@"%d:", hours]; minutes = (totalSeconds / 60) % 60; seconds = totalSeconds % 60; [formattedTimeString appendFormat:@"%02d:%02d", minutes, seconds]; 

And then, of course, in viewDidLoad I create an instance of the formattedTimeString variable:

 formattedTimeString = [[NSMutableString alloc] initWithCapacity:8]; 

I did not do any save / release in the first code fragment because I did not think it was necessary, but I could be wrong. However, I am freed in the dealloc method, so I have to be fine.