1

Based on this SO question asked a few hours ago, I have decided to implement a swizzled method that will allow me to take a formatted NSString as the format arg into stringWithFormat, and have it not break when omitting one of the numbered arg references (%1$@, %2$@)

I have it working, but this is the first copy, and seeing as this method is going to be potentially called hundreds of thousands of times per app run, I need to bounce this off of some experts to see if this method has any red flags, major performance hits, or optimizations

#define NUMARGS(...)  (sizeof((int[]){__VA_ARGS__})/sizeof(int))
@implementation NSString (UAFormatOmissions)
+ (id)uaStringWithFormat:(NSString *)format, ... {  
    if (format != nil) {
        va_list args;
        va_start(args, format);

        // $@ is an ordered variable (%1$@, %2$@...)
        if ([format rangeOfString:@"$@"].location == NSNotFound) {
            //call apples method
            NSString *s = [[[NSString alloc] initWithFormat:format arguments:args] autorelease];
            va_end(args);
            return s;
        }

        NSMutableArray *newArgs = [NSMutableArray arrayWithCapacity:NUMARGS(args)];
        id arg = nil;
        int i = 1;
        while (arg = va_arg(args, id)) {
            NSString *f = [NSString stringWithFormat:@"%%%d\$\@", i];
            i++;
            if ([format rangeOfString:f].location == NSNotFound) continue;
            else [newArgs addObject:arg];
        }
        va_end(args);

        char *newArgList = (char *)malloc(sizeof(id) * [newArgs count]);
        [newArgs getObjects:(id *)newArgList];
        NSString* result = [[[NSString alloc] initWithFormat:format arguments:newArgList] autorelease];
        free(newArgList);
        return result;
    }
    return nil;
}

The basic algorithm is:

  1. search the format string for the %1$@, %2$@ variables by searching for %@
  2. if not found, call the normal stringWithFormat and return
  3. else, loop over the args
  4. if the format has a position variable (%i$@) for position i, add the arg to the new arg array
  5. else, don't add the arg
  6. take the new arg array, convert it back into a va_list, and call initWithFormat:arguments: to get the correct string.

The idea is that I would run all [NSString stringWithFormat:] calls through this method instead.

This might seem unnecessary to many, but click on to the referenced SO question (first line) to see examples of why I need to do this.

Ideas? Thoughts? Better implementations? Better Solutions?

Community
  • 1
  • 1
coneybeare
  • 33,113
  • 21
  • 131
  • 183
  • Why are you casting the results of `arrayWithCapacity:` and `stringWithFormat:`? – dreamlax Jun 01 '10 at 00:42
  • i was trying to get rid of some compilation warnings, they can be removed – coneybeare Jun 01 '10 at 00:45
  • Are you sure that your `NUMARGS` macro does what you think it does? – dreamlax Jun 01 '10 at 01:17
  • it is working for me… I think so. it just takes the size of the args pointer array, then divides by the size of a pointer to get the number of args – coneybeare Jun 01 '10 at 01:50
  • @coneybeare: I'm quite certain that if you were to NSLog the result of every `NUMARGS(args)`, you will get 1 each time. `__VA_ARGS__` expands to the arguments passed to the macro itself, and you're only providing one argument. You're inserting an object of type `va_list` into an array of `int`; if `va_list` is implemented as a pointer (I'm quite sure it is on most systems), you should be getting a warning about a conversion without an explicit cast. – dreamlax Jun 01 '10 at 02:26
  • i will check ………… You are correct. but that doesnt change anything. with how the function works as it is an NSMutableArray that is growing as I am adding objects to it. The function works like this and all the previously broken strings are no longer broken. The main purpose of this question is not to get the function working, but to throw it out there for analysis, optimization and to bounce the idea off of others Thank you for helping me see the args thing – coneybeare Jun 01 '10 at 02:37

2 Answers2

3

Whoa there!

Instead of screwing with a core method that you very probably will introduce subtle bugs into, instead just turn on "Static Analyzer" in your project options, and it will run every build - if you get the arguments wrong it will issue a compiler warning for you.

I appreciate your desire to make the application more robust but I think it very likely that re-writing this method will more likely break your application than save it.

Kendall Helmstetter Gelner
  • 74,769
  • 26
  • 128
  • 150
  • 2
    The format strings are coming from NSLocalizedString, thus they are not found at compile time. ok, then assuming I do not swizzle it and call this method only when necessary, are there any major issues with it? – coneybeare Jun 01 '10 at 15:11
1

How about defining your own interim method instead of using format specifiers and stringWithFormat:? For example, you could define your own method replaceIndexPoints: to look for ($1) instead of %1$@. You would then format your string and insert translated replacements independently. This method could also take an array of strings, with NSNull or empty strings at the indexes that don't exist in the “untranslated” string.

Your method could look like this (if it were a category method for NSMutableString):

- (void) replaceIndexPointsWithStrings:(NSArray *) replacements
{
    // 1. look for largest index in "self".
    // 2. loop from the beginning to the largest index, replacing each
    //    index with corresponding string from replacements array.
}

Here's a few issues that I see with your current implementation (at a glance):

  1. The __VA_ARGS__ thingy explained in the comments.
  2. When you use while (arg = va_arg(args, id)), you are assuming that the arguments are nil terminated (such as for arrayWithObjects:), but with stringWithFormat: this is not a requirement.
  3. I don't think you're required to escape the $ and @ in your string format in your arg-loop.
  4. I'm not sure this would work well if uaStringWithFormat: was passed something larger than a pointer (i.e. long long if pointers are 32-bit). This may only be an issue if your translations also require inserting unlocalised numbers of long long magnitude.
dreamlax
  • 93,976
  • 29
  • 161
  • 209
  • I am trying to avoid editing tons of code. My large project is right near completion and doing massive code changes at this level would not be good. This is why I thought a stringWithFormat swizzle would be better. – coneybeare Jun 01 '10 at 01:04
  • Thanks. This is essentially what I did for my solution, using the above method. I now do not do a swizzle but instead, explicitly call this method for the 50+ multi-positional-variable methods in my app. This way, all the normal stringWithFormat calls are unaffected while I get to save cash from the translators and save QA time by not having to change all the format orders, strings or anything other than the 50+ method calls. – coneybeare Jun 02 '10 at 02:22
  • @coneybeare: I think you made the right choice here. Swizzling methods is unique and interesting but you really have to be careful about using this technique, especially in production. It may have seemed like a lot of effort but now the application design does not require runtime hacks to work. – dreamlax Jun 02 '10 at 04:25