Java equals() should evaluate primitives first (#244)

This is an optimization for generated equals() methods that evaluates the cheaper primitive comparisons before the more expensive complex object comparisons. This change also renames Schema.isObjCPrimitiveType to the more general Schema.isPrimitiveType. It was already being used by both Objective-C and JavaScript, and it's logic can also be applied to Java.
pinterest · Aug 28, 2019 · 9d367a5 · 9d367a5
1 parent a27b144
commit 9d367a5
Show file tree

Hide file tree

Showing 15 changed files with 51 additions and 49 deletions.
diff --git a/Examples/Java/Sources/Everything.java b/Examples/Java/Sources/Everything.java
@@ -328,12 +328,20 @@ public boolean equals(Object o) {
             return false;
         }
         Everything that = (Everything) o;
-        return Objects.equals(this.arrayProp, that.arrayProp) &&
-        Objects.equals(this.booleanProp, that.booleanProp) &&
+        return Objects.equals(this.unsignedShortEnum, that.unsignedShortEnum) &&
+        Objects.equals(this.unsignedIntEnum, that.unsignedIntEnum) &&
+        Objects.equals(this.unsignedCharEnum, that.unsignedCharEnum) &&
+        Objects.equals(this.stringEnum, that.stringEnum) &&
+        Objects.equals(this.shortEnum, that.shortEnum) &&
+        Objects.equals(this.numberProp, that.numberProp) &&
+        Objects.equals(this.nsuintegerEnum, that.nsuintegerEnum) &&
+        Objects.equals(this.nsintegerEnum, that.nsintegerEnum) &&
+        Objects.equals(this.intProp, that.intProp) &&
+        Objects.equals(this.intEnum, that.intEnum) &&
         Objects.equals(this.charEnum, that.charEnum) &&
+        Objects.equals(this.booleanProp, that.booleanProp) &&
+        Objects.equals(this.arrayProp, that.arrayProp) &&
         Objects.equals(this.dateProp, that.dateProp) &&
-        Objects.equals(this.intEnum, that.intEnum) &&
-        Objects.equals(this.intProp, that.intProp) &&
         Objects.equals(this.listPolymorphicValues, that.listPolymorphicValues) &&
         Objects.equals(this.listWithListAndOtherModelValues, that.listWithListAndOtherModelValues) &&
         Objects.equals(this.listWithMapAndOtherModelValues, that.listWithMapAndOtherModelValues) &&
@@ -347,22 +355,14 @@ public boolean equals(Object o) {
         Objects.equals(this.mapWithObjectValues, that.mapWithObjectValues) &&
         Objects.equals(this.mapWithOtherModelValues, that.mapWithOtherModelValues) &&
         Objects.equals(this.mapWithPrimitiveValues, that.mapWithPrimitiveValues) &&
-        Objects.equals(this.nsintegerEnum, that.nsintegerEnum) &&
-        Objects.equals(this.nsuintegerEnum, that.nsuintegerEnum) &&
-        Objects.equals(this.numberProp, that.numberProp) &&
         Objects.equals(this.otherModelProp, that.otherModelProp) &&
         Objects.equals(this.polymorphicProp, that.polymorphicProp) &&
         Objects.equals(this.setProp, that.setProp) &&
         Objects.equals(this.setPropWithOtherModelValues, that.setPropWithOtherModelValues) &&
         Objects.equals(this.setPropWithPrimitiveValues, that.setPropWithPrimitiveValues) &&
         Objects.equals(this.setPropWithValues, that.setPropWithValues) &&
-        Objects.equals(this.shortEnum, that.shortEnum) &&
-        Objects.equals(this.stringEnum, that.stringEnum) &&
         Objects.equals(this.stringProp, that.stringProp) &&
         Objects.equals(this.type, that.type) &&
-        Objects.equals(this.unsignedCharEnum, that.unsignedCharEnum) &&
-        Objects.equals(this.unsignedIntEnum, that.unsignedIntEnum) &&
-        Objects.equals(this.unsignedShortEnum, that.unsignedShortEnum) &&
         Objects.equals(this.uriProp, that.uriProp);
     }
 

diff --git a/Examples/Java/Sources/Image.java b/Examples/Java/Sources/Image.java
@@ -73,9 +73,9 @@ public boolean equals(Object o) {
             return false;
         }
         Image that = (Image) o;
-        return Objects.equals(this.height, that.height) &&
-        Objects.equals(this.url, that.url) &&
-        Objects.equals(this.width, that.width);
+        return Objects.equals(this.width, that.width) &&
+        Objects.equals(this.height, that.height) &&
+        Objects.equals(this.url, that.url);
     }
 
     @Override

diff --git a/Examples/Java/Sources/Pin.java b/Examples/Java/Sources/Pin.java
@@ -150,7 +150,8 @@ public boolean equals(Object o) {
             return false;
         }
         Pin that = (Pin) o;
-        return Objects.equals(this.attribution, that.attribution) &&
+        return Objects.equals(this.inStock, that.inStock) &&
+        Objects.equals(this.attribution, that.attribution) &&
         Objects.equals(this.attributionObjects, that.attributionObjects) &&
         Objects.equals(this.board, that.board) &&
         Objects.equals(this.color, that.color) &&
@@ -160,7 +161,6 @@ public boolean equals(Object o) {
         Objects.equals(this.description, that.description) &&
         Objects.equals(this.uid, that.uid) &&
         Objects.equals(this.image, that.image) &&
-        Objects.equals(this.inStock, that.inStock) &&
         Objects.equals(this.link, that.link) &&
         Objects.equals(this.media, that.media) &&
         Objects.equals(this.note, that.note) &&

diff --git a/Examples/Java/Sources/User.java b/Examples/Java/Sources/User.java
@@ -107,10 +107,10 @@ public boolean equals(Object o) {
             return false;
         }
         User that = (User) o;
-        return Objects.equals(this.bio, that.bio) &&
+        return Objects.equals(this.emailFrequency, that.emailFrequency) &&
+        Objects.equals(this.bio, that.bio) &&
         Objects.equals(this.counts, that.counts) &&
         Objects.equals(this.createdAt, that.createdAt) &&
-        Objects.equals(this.emailFrequency, that.emailFrequency) &&
         Objects.equals(this.firstName, that.firstName) &&
         Objects.equals(this.uid, that.uid) &&
         Objects.equals(this.image, that.image) &&

diff --git a/Examples/Java/Sources/VariableSubtitution.java b/Examples/Java/Sources/VariableSubtitution.java
@@ -77,10 +77,10 @@ public boolean equals(Object o) {
             return false;
         }
         VariableSubtitution that = (VariableSubtitution) o;
-        return Objects.equals(this.allocProp, that.allocProp) &&
-        Objects.equals(this.copyProp, that.copyProp) &&
+        return Objects.equals(this.newProp, that.newProp) &&
         Objects.equals(this.mutableCopyProp, that.mutableCopyProp) &&
-        Objects.equals(this.newProp, that.newProp);
+        Objects.equals(this.copyProp, that.copyProp) &&
+        Objects.equals(this.allocProp, that.allocProp);
     }
 
     @Override

diff --git a/Sources/Core/JSADTExtension.swift b/Sources/Core/JSADTExtension.swift
@@ -30,7 +30,7 @@ extension JSModelRenderer {
         case .array(itemType: _): return "Array<*>"
         case .set(itemType: _): return "Set<*>"
         case .map(valueType: .none): return "{}"
-        case let .map(valueType: .some(valueType)) where valueType.isObjCPrimitiveType:
+        case let .map(valueType: .some(valueType)) where valueType.isPrimitiveType:
             return "{ +[string]: number } /* \(valueType.debugDescription) */"
         case let .map(valueType: .some(valueType)):
             return "{ +[string]: \(adtCaseTypeName(valueType)) }"

diff --git a/Sources/Core/JSFileRenderer.swift b/Sources/Core/JSFileRenderer.swift
@@ -15,20 +15,20 @@ extension JSFileRenderer {
         switch schema.schema {
         case .array(itemType: .none):
             return "Array<*>"
-        case let .array(itemType: .some(itemType)) where itemType.isObjCPrimitiveType:
+        case let .array(itemType: .some(itemType)) where itemType.isPrimitiveType:
             // JS primitive types are represented as number
             return "Array<number /* \(itemType.debugDescription) */>"
         case let .array(itemType: .some(itemType)):
             return "Array<\(typeFromSchema(param, itemType.nonnullProperty()))>"
         case .set(itemType: .none):
             return "Array<*>"
-        case let .set(itemType: .some(itemType)) where itemType.isObjCPrimitiveType:
+        case let .set(itemType: .some(itemType)) where itemType.isPrimitiveType:
             return "Array<number /* \(itemType.debugDescription)> */>"
         case let .set(itemType: .some(itemType)):
             return "Array<\(typeFromSchema(param, itemType.nonnullProperty()))>"
         case .map(valueType: .none):
             return "{}"
-        case let .map(valueType: .some(valueType)) where valueType.isObjCPrimitiveType:
+        case let .map(valueType: .some(valueType)) where valueType.isPrimitiveType:
             return "{ +[string]: number } /* \(valueType.debugDescription) */"
         case let .map(valueType: .some(valueType)):
             return "{ +[string]: \(typeFromSchema(param, valueType.nonnullProperty())) }"

diff --git a/Sources/Core/JavaModelRenderer.swift b/Sources/Core/JavaModelRenderer.swift
@@ -67,7 +67,9 @@ public struct JavaModelRenderer: JavaFileRenderer {
     }
 
     func renderModelEquals() -> JavaIR.Method {
-        let bodyEquals = transitiveProperties.map { param, _ in
+        let bodyEquals = transitiveProperties.sorted { arg1, _ in
+            arg1.1.schema.isPrimitiveType
+        }.map { param, _ in
             "Objects.equals(this." + Languages.java.snakeCaseToPropertyName(param) + ", that." + Languages.java.snakeCaseToPropertyName(param) + ")"
         }.joined(separator: " &&\n")
 

diff --git a/Sources/Core/ObjCFileRenderer.swift b/Sources/Core/ObjCFileRenderer.swift
@@ -19,20 +19,20 @@ extension ObjCFileRenderer {
         switch schema.schema {
         case .array(itemType: .none):
             return "NSArray *"
-        case let .array(itemType: .some(itemType)) where itemType.isObjCPrimitiveType:
+        case let .array(itemType: .some(itemType)) where itemType.isPrimitiveType:
             // Objective-C primitive types are represented as NSNumber
             return "NSArray<NSNumber /* \(itemType.debugDescription) */ *> *"
         case let .array(itemType: .some(itemType)):
             return "NSArray<\(typeFromSchema(param, itemType.nonnullProperty()))> *"
         case .set(itemType: .none):
             return "NSSet *"
-        case let .set(itemType: .some(itemType)) where itemType.isObjCPrimitiveType:
+        case let .set(itemType: .some(itemType)) where itemType.isPrimitiveType:
             return "NSSet<NSNumber /*> \(itemType.debugDescription) */ *> *"
         case let .set(itemType: .some(itemType)):
             return "NSSet<\(typeFromSchema(param, itemType.nonnullProperty()))> *"
         case .map(valueType: .none):
             return "NSDictionary *"
-        case let .map(valueType: .some(valueType)) where valueType.isObjCPrimitiveType:
+        case let .map(valueType: .some(valueType)) where valueType.isPrimitiveType:
             return "NSDictionary<NSString *, NSNumber /* \(valueType.debugDescription) */ *> *"
         case let .map(valueType: .some(valueType)):
             return "NSDictionary<NSString *, \(typeFromSchema(param, valueType.nonnullProperty()))> *"

diff --git a/Sources/Core/ObjCModelRenderer.swift b/Sources/Core/ObjCModelRenderer.swift
@@ -196,19 +196,19 @@ public struct ObjCModelRenderer: ObjCFileRenderer {
         let adtRoots = properties.flatMap { (param, prop) -> [ObjCIR.Root] in
             switch prop.schema {
             case let .oneOf(types: possibleTypes):
-                let objProps = possibleTypes.map { SchemaObjectProperty(schema: $0, nullability: $0.isObjCPrimitiveType ? nil : .nullable) }
+                let objProps = possibleTypes.map { SchemaObjectProperty(schema: $0, nullability: $0.isPrimitiveType ? nil : .nullable) }
                 return adtRootsForSchema(property: param, schemas: objProps)
             case let .array(itemType: .some(itemType)):
                 switch itemType {
                 case let .oneOf(types: possibleTypes):
-                    let objProps = possibleTypes.map { SchemaObjectProperty(schema: $0, nullability: $0.isObjCPrimitiveType ? nil : .nullable) }
+                    let objProps = possibleTypes.map { SchemaObjectProperty(schema: $0, nullability: $0.isPrimitiveType ? nil : .nullable) }
                     return adtRootsForSchema(property: param, schemas: objProps)
                 default: return []
                 }
             case let .map(valueType: .some(additionalProperties)):
                 switch additionalProperties {
                 case let .oneOf(types: possibleTypes):
-                    let objProps = possibleTypes.map { SchemaObjectProperty(schema: $0, nullability: $0.isObjCPrimitiveType ? nil : .nullable) }
+                    let objProps = possibleTypes.map { SchemaObjectProperty(schema: $0, nullability: $0.isPrimitiveType ? nil : .nullable) }
                     return adtRootsForSchema(property: param, schemas: objProps)
                 default: return []
                 }

diff --git a/Sources/Core/ObjectiveCDictionaryExtension.swift b/Sources/Core/ObjectiveCDictionaryExtension.swift
@@ -16,7 +16,7 @@ extension ObjCModelRenderer {
             !schema.schema.isBoolean()
         }.map { (param, schemaObj) -> String in
             ObjCIR.ifStmt("_" + "\(self.dirtyPropertiesIVarName).\(dirtyPropertyOption(propertyName: param, className: self.className))") { [
-                schemaObj.schema.isObjCPrimitiveType ?
+                schemaObj.schema.isPrimitiveType ?
                     self.renderAddToDictionaryStatement(.ivar(param), schemaObj.schema, dictionary) :
                     ObjCIR.ifElseStmt("_\(Languages.objectiveC.snakeCaseToPropertyName(param)) != nil") { [
                         self.renderAddToDictionaryStatement(.ivar(param), schemaObj.schema, dictionary),
@@ -202,7 +202,7 @@ extension ObjCFileRenderer {
             }
             let currentResult = "result\(counter)"
             let currentObj = "obj\(counter)"
-            let itemClass = itemType.isObjCPrimitiveType ? "id" : typeFromSchema(param, itemType.nonnullProperty())
+            let itemClass = itemType.isPrimitiveType ? "id" : typeFromSchema(param, itemType.nonnullProperty())
             return [
                 "__auto_type items\(counter) = \(propIVarName);",
                 "\(CollectionClass.array.mutableName()) *\(currentResult) = [\(CollectionClass.array.mutableName()) \(CollectionClass.array.initializer())items\(counter).count];",

diff --git a/Sources/Core/ObjectiveCEqualityExtension.swift b/Sources/Core/ObjectiveCEqualityExtension.swift
@@ -90,7 +90,7 @@ extension ObjCFileRenderer {
 
         // Performance optimization - compare primitives before resorting to more expensive `isEqual` calls
         let sortedProps = properties.sorted { arg1, _ in
-            arg1.1.schema.isObjCPrimitiveType
+            arg1.1.schema.isPrimitiveType
         }
 
         let propReturnStmts = sortedProps.map { param, prop -> String in

diff --git a/Sources/Core/ObjectiveCIR.swift b/Sources/Core/ObjectiveCIR.swift
@@ -149,15 +149,6 @@ extension SchemaObjectRoot {
 }
 
 extension Schema {
-    var isObjCPrimitiveType: Bool {
-        switch self {
-        case .boolean, .integer, .enumT, .float:
-            return true
-        default:
-            return false
-        }
-    }
-
     func memoryAssignmentType() -> ObjCMemoryAssignmentType {
         switch self {
         // Use copy for any string, date, url etc.

diff --git a/Sources/Core/ObjectiveCInitExtension.swift b/Sources/Core/ObjectiveCInitExtension.swift
@@ -79,7 +79,7 @@ extension ObjCModelRenderer {
                 let currentResult = "result\(counter)"
                 let currentTmp = "tmp\(counter)"
                 let currentObj = "obj\(counter)"
-                if itemType.isObjCPrimitiveType {
+                if itemType.isPrimitiveType {
                     return [
                         "\(propertyToAssign) = \(rawObjectName);",
                     ]
@@ -104,7 +104,7 @@ extension ObjCModelRenderer {
                 let currentTmp = "tmp\(counter)"
                 let currentObj = "obj\(counter)"
 
-                if itemType.isObjCPrimitiveType {
+                if itemType.isPrimitiveType {
                     return [
                         "NSArray *items = \(rawObjectName);",
                         "\(propertyToAssign) = [NSSet setWithArray:items];",
@@ -125,7 +125,7 @@ extension ObjCModelRenderer {
                     ] },
                     "\(propertyToAssign) = \(currentResult);",
                 ]
-            case let .map(valueType: .some(valueType)) where valueType.isObjCPrimitiveType == false:
+            case let .map(valueType: .some(valueType)) where valueType.isPrimitiveType == false:
                 let currentResult = "result\(counter)"
                 let currentItems = "items\(counter)"
                 let (currentKey, currentObj, currentStop) = ("key\(counter)", "obj\(counter)", "stop\(counter)")

diff --git a/Sources/Core/Schema.swift b/Sources/Core/Schema.swift
@@ -164,6 +164,15 @@ public indirect enum Schema {
     case oneOf(types: [Schema]) // ADT
     case enumT(EnumType)
     case reference(with: URLSchemaReference)
+
+    var isPrimitiveType: Bool {
+        switch self {
+        case .boolean, .integer, .enumT, .float:
+            return true
+        default:
+            return false
+        }
+    }
 }
 
 typealias Property = (Parameter, SchemaObjectProperty)
@@ -306,7 +315,7 @@ extension Schema {
                         }
                         return (key, schemaOpt)
                     }.map { name, optSchema in optSchema.map {
-                        (name, SchemaObjectProperty(schema: $0, nullability: $0.isObjCPrimitiveType ? nil : requiredProps.contains(name) ? .nonnull : .nullable))
+                        (name, SchemaObjectProperty(schema: $0, nullability: $0.isPrimitiveType ? nil : requiredProps.contains(name) ? .nonnull : .nullable))
                     }
                     }
                     let lifted: [Property]? = optTuples.reduce([], { (build: [Property]?, tupleOption: Property?) -> [Property]? in