Personally, I wouldn't change any native code at this point unless it was fixing a crash. The review is for 8u40, correct?

Steve

On 2014-07-02, 5:38 PM, Chris Bensen wrote:
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.fer...@oracle.com <mailto:danno.fer...@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/ <http://cr.openjdk.java.net/%7Eshemnon/RT-37788/webrev.00/>
JIRA: https://javafx-jira.kenai.com/browse/RT-37788

—Danno


Reply via email to