CODEWITHSUNDEEP
objective-cnsdictionaryimmutabilitynsmutabledictionary
Tyilo
Tyilo

Reputation: 30102

Is it bad practice to return a mutable object when the return value is an immutable object?

If I have this method:

+ (NSDictionary *)dictionaryFromQueryString:(NSString *)queryString
{
    NSMutableDictionary *this = [[NSMutableDictionary alloc] init];

    NSArray *fields = [queryString componentsSeparatedByString:@"&"];

    for(NSString *field in fields)
    {
        NSArray *fieldParts = [field componentsSeparatedByString:@"="];

        NSString *value = @"";

        if(fieldParts.count > 1)
        {
            value = [[fieldParts subarrayWithRange:NSMakeRange(1, fieldParts.count - 1)] componentsJoinedByString:@"="];
        }

        [this setObject:unescape(value) forKey:unescape(fieldParts[0])];
    }

    return this;
}

Is it then bad practice that I return a NSMutableDictionary instead of a NSDictionary?

Should I convert it to a NSDictionary with return [this copy];?

Upvotes: 3

Views: 239

Answers (3)

Simple Coder
Simple Coder

Reputation: 1

Returning an immutable object this way isn't really bad practice, because NSMutableDictionary is a subclass of NSDictionary. This is polymorphism, so it's 'all good.'

But I would probably return an autoreleased copy like this anyway:

return [NSDictionary dictionaryWithDictionary:this];

Upvotes: 0

bbum
bbum

Reputation: 162712

It depends.

Sergio's answer is correct, save for one very important issue:

What happens when your object that contains the mutable dictionary mutates the dictionary after another object retrieves it? Unless that other object is written specifically to support the potential that the dictionary might mutate, the other object is now going to be in an inconsistent state.

Given that copy is fast for a dictionary as it is a shallow immutable copy, you are generally far better off always returning a copy than returning a reference to the mutable version. If you find that your code is pounding on the method that makes a copy, then cache an immutable copy in your object and vend that, invalidating it whenever the mutable backing store changes.

Upvotes: 5

sergio
sergio

Reputation: 69027

I don't think it is bad practice. The net effect of doing this is that the receiver of your NSDictionary will not try to modify the object (although the object is mutable). This is perfectly safe and it makes sense since your consumer method is kept more general (it can work both with mutable and non mutable objects).

Upvotes: 2

Related Questions