1

I have a method updateUserPlaceDictionary that draws text from some text fields and throws it in an NSDictionary. After noticing that all the values for the dictionary were null, i tried manually setting some strings for its keys, like so:

- (void)updatePlaceDictionary {
    //Create a dictionary that holds the location data.
    [self.placeDictionary setValue:[NSString stringWithFormat:@"155 Bovet Rd"] forKey:@"Street"];
    [self.placeDictionary setValue:[NSString stringWithFormat:@"san mateo"] forKey:@"City"];
    [self.placeDictionary setValue:[NSString stringWithFormat:@"ca"] forKey:@"State"];
    [self.placeDictionary setValue:[NSString stringWithFormat:@"94402"] forKey:@"ZIP"];

    NSLog(@"%@, %@, %@, %@", [self.placeDictionary objectForKey:@"Street"],
          [self.placeDictionary objectForKey:@"City"],
          [self.placeDictionary objectForKey:@"State"],
          [self.placeDictionary objectForKey:@"ZIP"]);
}

Here is my declaration of placeDictionary:

@property NSMutableDictionary* placeDictionary;

Also, i did make sure to synthesize it in the .m file.

I have the method log to the console all the location data that was put into the dictionary but all I get are null values. I have the same exact function in another view controller that works completely fine. Can someone tell me if they see anything improper?

Sam D20
  • 2,535
  • 5
  • 25
  • 43

2 Answers2

10

You have a comment stating

//Create a dictionary that holds the location data.

but you never actually create it. Initialize the dictionary and you'll be fine.

self.placeDictionary = [NSMutableDictionary dictionary];

Also beware: setValue:forKey: is not the same as setObject:forKey:. Read this answer to know more on the subject, but in general you should use setObject:forKey: for dictionaries, e.g.

[self.placeDictionary setObject:@"155 Bovet Rd" forKey:@"Street"];

or the modern version of the syntax:

self.placeDictionary[@"Street"] = @"155 Bovet Rd";

Using Objective-C literals you can actually make the whole code a lot nicer, check it out:

self.placeDictionary = @{
    @"Street": @"155 Bovet Rd",
    @"City"  : @"san mateo",
    @"State" : @"ca",
    @"ZIP"   : @"94402"
};

Please note that the literal syntax produces a non-mutable dictionary and it may not fit your needs.

As a final remark, please don't use

[NSString stringWithFormat:@"aString"]

since is semantically equivalent to

@"aString"

but less optimized in terms of performances (not to mention code quality).

Community
  • 1
  • 1
Gabriele Petronella
  • 106,943
  • 21
  • 217
  • 235
  • 1
    A high quality answer. Not only fixed my problem but provided info that will be useful in the future. Will try to remember. Thanks buddy! – Sam D20 Aug 29 '13 at 00:21
2

It seems as you didn't allocate it, you can do it in the getter.

-(NSMutableDictionary *)placeDictionary
{
    if (!_placeDictionary) {
        _placeDictionary = [NSMutableDictionary dictionary];
    }
    return _placeDictionary;
}
Moxy
  • 4,162
  • 2
  • 30
  • 49
  • An initialization method would be a better place. – Chuck Aug 29 '13 at 00:07
  • By doing it lazily you don't have to worry when and where you'll be calling it first in your code. – Moxy Aug 29 '13 at 00:09
  • 5
    By putting it in `init`, you also don't have to worry about that unless you're running code before `init`. And you don't unnecessarily bloat your code with verbose getters. – Chuck Aug 29 '13 at 00:12
  • and you don't need to override the getter – Gabriele Petronella Aug 29 '13 at 00:14
  • So you just allocate everything upfront in init before using it? – Moxy Aug 29 '13 at 00:14
  • 1
    that's why is called init. You initialize the object state. – Gabriele Petronella Aug 29 '13 at 00:16
  • 1
    A dictionary wouldn't make the difference so it's not relevant to this question. But there is a reason why lazy instantiation exists. And I have to disagree about the init method, you shouldn't initialize all your object state only because that method is called init. – Moxy Aug 29 '13 at 00:20
  • 1
    @Moxy: Absolutely, there is a reason why lazy initialization exists. It is useful for delaying particularly expensive operations that are not necessary for core functionality. There just isn't any reason to use it here. All it adds is verbosity and inefficiency. – Chuck Aug 29 '13 at 00:29
  • @Chuck I don't see any negative effect apart from some more lines of code that are anyway light/easy to read which makes it a coding style choice. So why would it be a non sensible idea? Of course I'm not saying it's the only solution nor the best. But I can't understand why it's bad behavior. – Moxy Aug 29 '13 at 00:37