Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Based on a python name generator grammar I found here I've decided to write my own in objective-C. The idea is I can load the grammar from a plist file and generate random names from it.

I'm looking for a general code review and any comments and or suggestions.

NameGenerator.h:

@interface NameGenerator : NSObject
- (id)initWithGrammar:(NSString *)plistName;
- (NSString*)name;
@end

NameGenerator.m:

@interface NameGenerator()
@property (nonatomic,strong) NSDictionary *grammar;
-(NSString*)replaceKey:(NSString*)input;
@end

@implementation NameGenerator
@synthesize grammar = _grammar;

- (id)init {
    @throw [NSException exceptionWithName: @"NameGeneratorInit"
                                   reason: @"-init is not allowed, use -initWithGrammar: instead"
                                 userInfo: nil];
}

- (id)initWithGrammar:(NSString *)plistName
{
    self = [super init];
    if (self) {
        NSString* plistPath = [[NSBundle mainBundle] pathForResource:plistName ofType:@"plist"];
        self.grammar = [NSDictionary dictionaryWithContentsOfFile:plistPath];

        id object = [self.grammar objectForKey:@"grammar"];
        if (![object isKindOfClass:[NSString class]]) {
            NSLog(@"initWithGrammar: Not a grammar plist");
            self = nil;
        }

        NSString *ver = object;
        if (![ver isEqualToString:@"NG.CF.1"]) {
            NSLog(@"initWithGrammar: plist version is wrong");
            self = nil;
        }
    }
    return self;
}

- (NSString*)name {
    return [self replaceKey:@"name"];
}

-(NSString*)replaceKey:(NSString*)input
{
    NSString *output = @"";
    NSArray *parts = [input componentsSeparatedByString:@","];

    if ([parts count] > 1) {
        for(NSString *part in parts) {
            output = [output stringByAppendingString:[self replaceKey:part]];
        }
    } else {
        id object = [self.grammar objectForKey:input];
        if ([object isKindOfClass:[NSString class]]) {
            output = [self replaceKey:object];
        }
        else if ([object isKindOfClass:[NSArray class]]) {
            NSUInteger randomIndex = arc4random() % [object count];
            output = [self replaceKey:[object objectAtIndex:randomIndex]];
        }
        else {
            output = input;
        }
    }

    return output;
}

ork.plist:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>grammar</key>
    <string>NG.CF.1</string>
    <key>name</key>
    <string>nameStart,nameMiddle0to3,nameEnd</string>
    <key>nameMiddle0to3</key>
    <array>
        <string></string>
        <string>nameMiddle</string>
        <string>nameMiddle,nameMiddle</string>
        <string>nameMiddle,nameMiddle,nameMiddle</string>
    </array>
    <key>nameStart</key>
    <array>
        <string>nsCons,tick,nsCons,nmVowel</string>
        <string>nsCons,nmVowel</string>
        <string>nsCons,nmVowel</string>
        <string>nsCons,nmVowel</string>
        <string>nsVowel</string>
    </array>
    <key>nameMiddle</key>
    <string>nmCons,nmVowel</string>
    <key>nameEnd</key>
    <array>
        <string>neCons,neVowel</string>
        <string>neCons</string>
        <string>neCons</string>
    </array>
    <key>nsCons</key>
    <array>
        <string>d</string>
        <string>g</string>
        <string>k</string>
        <string>t</string>
        <string>gr</string>
    </array>
    <key>nmCons</key>
    <array>
        <string>d</string>
        <string>g</string>
        <string>k</string>
        <string>t</string>
        <string>r</string>
        <string>s</string>
        <string>z</string>
        <string>kt</string>
        <string>rs</string>
        <string>gr</string>
    </array>
    <key>neCons</key>
    <array>
        <string>r</string>
        <string>s</string>
        <string>z</string>
    </array>
    <key>nsVowel</key>
    <array>
        <string>e</string>
        <string>u</string>
    </array>
    <key>nmVowel</key>
    <array>
        <string>a</string>
        <string>e</string>
        <string>i</string>
        <string>o</string>
        <string>u</string>
    </array>
    <key>neVowel</key>
    <array>
        <string>a</string>
        <string>u</string>
    </array>
    <key>tick</key>
    <array>
        <string>&apos;</string>
    </array>
</dict>
</plist>
share|improve this question

1 Answer

up vote 1 down vote accepted

It generally looks like good code to me, so these come under "nits":

  • In -initWithGrammar: you're actually passing the name of the grammar, not the grammar itself. Perhaps -initWithGrammarName: would be better.

  • It's conventional not to use accessors in initialisers or destructors, because the object isn't in a consistent state during these methods. This means replacing self.grammar = with _grammar = in the initialiser.

  • Given the change made above, why does the grammar property need to be readwrite? Customers of your class pass a filename into the initialiser and then, presumably, never change the constructed dictionary so it can be readonly.

  • in replaceKey:, if the plist happens to contain a number or a dictionary value for the key then you'll return that instead of an NSString. I realise the grammar format isn't supposed to do that but it's good form to assert this so that when it does happen, people find out the easy way.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.