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

Question

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];?

Answer 1

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.

Answer 2

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).