Review Request: RT-37788

[Chris Bensen]

I’m not sure about for 8u20. Seems fairly straight forward, and your Obj-C seems as good as any Obj-C. My only complaint at the moment is the following:
 358     if ([pathParts count] > 2) {
 359         // for 3 or more steps, the domain is first.second.third and the keys are "/first/second/third/", "fourth/", "fifth/"... etc
 360         persistentDomain = [NSString stringWithFormat: @"%@.%@.%@", [pathParts objectAtIndex: 0],
 361                             [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]];
 362         
 363         [dictPath replaceObjectAtIndex: 0 withObject: [NSString stringWithFormat:@"/%@/%@/%@", [pathParts objectAtIndex: 0],
 364                                                        [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]]];
 365         [dictPath removeObjectAtIndex: 2];
 366         [dictPath removeObjectAtIndex: 1];
 367     } else {
 368         // for 1 or two steps, the domain is first.second.third and the keys are "/", "first/", "second/"
 369         persistentDomain = @DEFAULT_JAVA_PREFS_DOMAIN;
 370         [dictPath insertObject: @"" atIndex:0];
 371     }

what if [pathParts count] is 0? I’d probably do a switch:

switch ([pathParts count]) {
  case 0:
     //error
     return/break;
  case 1:
  case 2:
 368         // for 1 or two steps, the domain is first.second.third and the keys are "/", "first/", "second/"
 369         persistentDomain = @DEFAULT_JAVA_PREFS_DOMAIN;
 370         [dictPath insertObject: @"" atIndex:0];
  default:
 359         // for 3 or more steps, the domain is first.second.third and the keys are "/first/second/third/", "fourth/", "fifth/"... etc
 360         persistentDomain = [NSString stringWithFormat: @"%@.%@.%@", [pathParts objectAtIndex: 0],
 361                             [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]];
 362         
 363         [dictPath replaceObjectAtIndex: 0 withObject: [NSString stringWithFormat:@"/%@/%@/%@", [pathParts objectAtIndex: 0],
 364                                                        [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]]];
 365         [dictPath removeObjectAtIndex: 2];
 366         [dictPath removeObjectAtIndex: 1];

}


Make sense? Clear as mud?

Chris


On Jul 2, 2014, at 2:15 PM, Danno Ferrin <danno.ferrin at oracle.com> wrote:

> Chris, Kevin, Steve,
> 
> Please review this fix for RT-37788.  Since I am not an objective C any comments are welcome.  Also, please consider if this is too much for an 8u20 fix (the diff is against the current 8u40 codebase).
> 
> Webrev: http://cr.openjdk.java.net/~shemnon/RT-37788/webrev.00/
> JIRA: https://javafx-jira.kenai.com/browse/RT-37788
> 
> —Danno