Optimize IGListDiffable for common diffIdentifier types

Summary: Let's use simple NSNumber wrapping to make diffable objects for most primitive types. This should be faster than building strings. Reviewed By: calimarkus Differential Revision: D10416548 fbshipit-source-id: af5b0a5afc6cd2c45c7e0bf2aa872864c12cbd98
GitHawkApp · Oct 23, 2018 · 0efb259 · 0efb259
1 parent 2b4ee9b
commit 0efb259
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 21 deletions.
diff --git a/remodel-plugin/features/iglistdiffable.feature b/remodel-plugin/features/iglistdiffable.feature
@@ -151,7 +151,7 @@ Feature: Outputting Value Objects implementing IGListDiffable
 
       - (id<NSObject>)diffIdentifier
       {
-        return NSStringFromCGRect(_someRect);
+        return [NSValue valueWithCGRect:_someRect];
       }
 
       - (NSUInteger)hash
@@ -191,6 +191,24 @@ Feature: Outputting Value Objects implementing IGListDiffable
 
       """
 
+  Scenario: Generating a value object, which correctly implements IGListDiffable using an NSInteger property
+    Given a file named "project/values/Test.value" with:
+      """
+      Test includes(IGListDiffable) {
+        %diffIdentifier
+        NSInteger count
+      }
+      """
+    When I run `../../bin/generate project`
+    Then the file "project/values/Test.m" should contain:
+      """
+      - (id<NSObject>)diffIdentifier
+      {
+        return @(_count);
+      }
+
+      """
+
   Scenario: Generating a value object, which correctly implements IGListDiffable defaulting to self as diffIdentifier
     Given a file named "project/values/Test.value" with:
       """

diff --git a/remodel-plugin/src/__tests__/plugins/iglistdiffable-test.ts b/remodel-plugin/src/__tests__/plugins/iglistdiffable-test.ts
@@ -164,7 +164,7 @@ describe('ObjectSpecPlugins.IGListDiffable', function() {
 
         const expectedInstanceMethods:ObjC.Method[] = [
           igListDiffableIsEqualMethod(),
-          igListDiffableDiffIdentifierMethodWithCode('return [NSString stringWithFormat:@"%lld", (long long)_age];')
+          igListDiffableDiffIdentifierMethodWithCode('return @(_age);')
         ];
 
         expect(instanceMethods).toEqualJSON(expectedInstanceMethods);
@@ -203,7 +203,7 @@ describe('ObjectSpecPlugins.IGListDiffable', function() {
 
         const expectedInstanceMethods:ObjC.Method[] = [
           igListDiffableIsEqualMethod(),
-          igListDiffableDiffIdentifierMethodWithCode('return NSStringFromCGRect(_rect);')
+          igListDiffableDiffIdentifierMethodWithCode('return [NSValue valueWithCGRect:_rect];')
         ];
 
         expect(instanceMethods).toEqualJSON(expectedInstanceMethods);
@@ -256,7 +256,7 @@ describe('ObjectSpecPlugins.IGListDiffable', function() {
 
         const expectedInstanceMethods:ObjC.Method[] = [
           igListDiffableIsEqualMethod(),
-          igListDiffableDiffIdentifierMethodWithCode('return [NSString stringWithFormat:@"%lld", (long long)_age];')
+          igListDiffableDiffIdentifierMethodWithCode('return @(_age);')
         ];
 
         expect(instanceMethods).toEqualJSON(expectedInstanceMethods);

diff --git a/remodel-plugin/src/plugins/iglistdiffable-utils.ts b/remodel-plugin/src/plugins/iglistdiffable-utils.ts
@@ -44,6 +44,10 @@ function nullableObjectValueWithFallback(objectValue:string, optionalFallback:st
   return (optionalFallback === null) ? objectValue : `${objectValue} ?: ${optionalFallback}`;
 }
 
+function wrappedInNSValueForTypeName(iVarString:string, typeName:string) {
+  return `[NSValue valueWith${typeName}:${iVarString}]`;
+}
+
 function objectValueForAttribute(attribute:ObjectSpec.Attribute, optionalFallback:string=null):string {
   const iVarString:string = ObjectSpecCodeUtils.ivarForAttribute(attribute);
   const type:ObjC.Type = ObjectSpecCodeUtils.computeTypeOfAttribute(attribute);
@@ -56,58 +60,58 @@ function objectValueForAttribute(attribute:ObjectSpec.Attribute, optionalFallbac
       return nullableObjectValueWithFallback(iVarString, optionalFallback);
     },
     BOOL: function() {
-      return iVarString + " ? @\"YES\" : @\"NO\"";
+      return `@(${iVarString})`;
     },
     NSInteger: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%lld", "long long");
+      return `@(${iVarString})`;
     },
     NSUInteger: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%llu", "unsigned long long");
+      return `@(${iVarString})`;
     },
     double: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%lf");
+      return `@(${iVarString})`;
     },
     float: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%f");
+      return `@(${iVarString})`;
     },
     CGFloat: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%f");
+      return `@(${iVarString})`;
     },
     NSTimeInterval: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%lf");
+      return `@(${iVarString})`;
     },
     uintptr_t: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%ld");
+      return `@(${iVarString})`;
     },
     uint32_t: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%u");
+      return `@(${iVarString})`;
     },
     uint64_t: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%llu");
+      return `@(${iVarString})`;
     },
     int32_t: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%d");
+      return `@(${iVarString})`;
     },
     int64_t: function() {
-      return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%lld");
+      return `@(${iVarString})`;
     },
     SEL: function() {
       return functionReturnValueForIvarWithFunctionName(iVarString, "NSStringFromSelector");
     },
     NSRange: function() {
-      return functionReturnValueForIvarWithFunctionName(iVarString, "NSStringFromRange");
+      return wrappedInNSValueForTypeName(iVarString, 'Range');
     },
     CGRect: function() {
-      return functionReturnValueForIvarWithFunctionName(iVarString, "NSStringFromCGRect");
+      return wrappedInNSValueForTypeName(iVarString, type.name);
     },
     CGPoint: function() {
-      return functionReturnValueForIvarWithFunctionName(iVarString, "NSStringFromCGPoint");
+      return wrappedInNSValueForTypeName(iVarString, type.name);
     },
     CGSize: function() {
-      return functionReturnValueForIvarWithFunctionName(iVarString, "NSStringFromCGSize");
+      return wrappedInNSValueForTypeName(iVarString, type.name);
     },
     UIEdgeInsets: function() {
-      return functionReturnValueForIvarWithFunctionName(iVarString, "NSStringFromUIEdgeInsets");
+      return wrappedInNSValueForTypeName(iVarString, type.name);
     },
     Class: function() {
       return formattedStringValueForIvarWithFormatSpecifier(iVarString, "%@");