Memory Leaks - Formatting a String To Display Time, Each Second

Question

Hey guys. I have a method that gets called each second which I want to use to display the time that my app has been doing it's work. Currently the class I'm using (Which I did not create) has a property named progress which stores the total number of seconds.

I have already written some code which takes these seconds and formats it into a readable string. I'm new to this, so pardon me if it's 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 appear as "3:35" for example to show 3 minutes, 35 seconds. I only want to show the hour section if it has been an hour, in which case it would be "2:3:35" for example (Can anyone recommend a better way to format this?).

The problem I am having is where I actually create/set the string (The lines tagged WRONG). Since this is being done every second, I would easily get a leak if I keep asking for a new string object. I figure I can solve this by releasing the foramttedTimeString at the end of the method, but is this the correct way to accomplish this? Would an NSMutableString help in any way? Is there a better, Cocoa way of doing this? I already asked in #iphonedev @ freenode and they said I would have to write this method myself, but I figured I'd ask again.

To provide context: this is an internet radio streaming app (I know there are many already, but I'm just practicing). I want to be able to show the amount of time the stream has been playing for.

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

Answer 1

Hey Blaenk,

Your code looks pretty good. You're not leaking any memory because the string objects you create have a retain count of zero and will be cleaned up by the system. However, if formattedTimeString is not a local variable in your function, you need to retain it at the end to prevent this from happening! To do that, you would add [formattedTimeString retain] to the end of your code block, and then before replacing the string object you would add [formattedTimeString release].

As a general rule, functions with names containing "alloc", "copy", "create", and "new" return objects that have already been retained, (meaning their retain count is +1). It's your responsibility to call release or autorelease on these objects when you're done using them - or they will just start piling up in memory.

Functions like "stringWithFormat:", "imageNamed:", and "arrayWithCapacity:" all return objects with a retain count of zero - so you can safely discard them (as you are in the code sample). If you want to keep them around, you should call retain to make sure they are not cleaned up while you're using them.

All that said, I think the main problem is your use of stringByAppendingFormat:. Since the NSString you're using isn't mutable, that call returns a new string. You'd want to say:

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

Alternatively, you could use an NSMutableString. Since this is something you'll be doing over and over again, I'd recommend doing that. Technically, either way is fine though.

Hope that helps! The whole retain/release thing can get confusing. Just remember that each object has a "retainCount" and once it hits zero there's no telling what happens to the object or it's data.

Answer 2

I would do it something like:

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 at the end, formattedTimeString is the desired time, but you do not "own" it - you must retain it, or store it in a "copy" property if you wish to keep it around.

Note that the %02d gives you a guarenteed two digits, zero filled number, which is usually what you want for numbers in parts of times.

To see how you would do it with stringByAppendingFormat, it would 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'll get times like 3:4:05, rether than a more desirable 3:04:05.

Note that formattedTimeString is being overwritten each time, but that is OK bvecause you do not "own" it at any time, so you are not 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 time result is the undesirable 3:4:05, and again you do not own formattedTimeString at the end, so it must be retained or stored with a copy property to keep it around.

Answer 3

Hey thanks guys I appreciate the responses.

I ended up doing this, and it works, but I would like to know if you guys see any 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 instantiate the instance variable formattedTimeString:

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

I did not do any retaining/releasing in the first code snippet because I didn't think it was necessary, but I could be wrong. I am, however, releasing in the dealloc method, so I should be fine there.

Answer 4

For knowing the deltas as time units, 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];

Then you can reference the parts with something like this [components minute]. Remember, you'll have to release the calendar in dealloc.