From faeeabddecba950d3db5aec284e95857308de78b Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Wed, 28 Aug 2019 14:10:16 -0700 Subject: [PATCH] Java equals() should evaluate primitives first 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. --- Examples/Java/Sources/Everything.java | 24 +++++++++---------- Examples/Java/Sources/Image.java | 6 ++--- Examples/Java/Sources/Pin.java | 4 ++-- Examples/Java/Sources/User.java | 4 ++-- .../Java/Sources/VariableSubtitution.java | 6 ++--- Sources/Core/JSADTExtension.swift | 2 +- Sources/Core/JSFileRenderer.swift | 6 ++--- Sources/Core/JavaModelRenderer.swift | 4 +++- Sources/Core/ObjCFileRenderer.swift | 6 ++--- Sources/Core/ObjCModelRenderer.swift | 6 ++--- .../Core/ObjectiveCDictionaryExtension.swift | 4 ++-- .../Core/ObjectiveCEqualityExtension.swift | 2 +- Sources/Core/ObjectiveCIR.swift | 9 ------- Sources/Core/ObjectiveCInitExtension.swift | 6 ++--- Sources/Core/Schema.swift | 11 ++++++++- 15 files changed, 51 insertions(+), 49 deletions(-) diff --git a/Examples/Java/Sources/Everything.java b/Examples/Java/Sources/Everything.java index 51d9b479..b7bb07e8 100644 --- 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 index de6a46da..aecf0b0e 100644 --- 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 index 130b0cfa..0e6eca8c 100644 --- 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 index f238197f..3017bacf 100644 --- 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 index d8bc545e..6409c721 100644 --- 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 index 1bdae423..a5eb5b3b 100644 --- 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 index 159e61af..078f02fa 100644 --- 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" 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 */>" 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 index b521d3eb..2d05b111 100644 --- 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 index c124bea0..cec7f0ae 100644 --- 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 *" 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 \(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 *" case let .map(valueType: .some(valueType)): return "NSDictionary *" diff --git a/Sources/Core/ObjCModelRenderer.swift b/Sources/Core/ObjCModelRenderer.swift index 687a3f4c..c8490b75 100644 --- 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 index 392f3b3e..7ca379ea 100644 --- 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 index 9ecf2562..2c3ce07b 100644 --- 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 index 504c5e7d..1a3c452f 100644 --- 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 index fead49db..b8600311 100644 --- 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 index be19d269..78bb6a41 100644 --- 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