perf(core): remove usage of arrays while forming duplicate item errors (

#804) * refactor: don't use arrays to check uniqueness Signed-off-by: Ertugrul Karademir <[email protected]> * refactor: also refactor unique property name check Signed-off-by: Ertugrul Karademir <[email protected]> * refactor: remove array for decorator uniqueness check Signed-off-by: Ertugrul Karademir <[email protected]> * refactor: remove uniqueness check from scalar declarations as well Signed-off-by: Ertugrul Karademir <[email protected]> --------- Signed-off-by: Ertugrul Karademir <[email protected]>
accordproject · Feb 6, 2024 · e1527cf · e1527cf
1 parent 0e363fb
commit e1527cf
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 58 deletions.
diff --git a/packages/concerto-core/lib/introspect/classdeclaration.js b/packages/concerto-core/lib/introspect/classdeclaration.js
@@ -255,24 +255,21 @@ class ClassDeclaration extends Declaration {
         }
         // we also have to check fields defined in super classes
         const properties = this.getProperties();
-        const propertyFieldNames = properties.map(
-            d => d.getName()
-        );
-        const uniquePropertyFieldNames = new Set(propertyFieldNames);
-
-        if (uniquePropertyFieldNames.size !== properties.length) {
-            const duplicateElements = propertyFieldNames
-                .filter(
-                    (item, index) => propertyFieldNames.indexOf(item) !== index
+        const uniquePropertyNames = new Set();
+        properties.forEach(p => {
+            const propertyName = p.getName();
+            if (!uniquePropertyNames.has(propertyName)) {
+                uniquePropertyNames.add(propertyName);
+            } else {
+                const formatter = Globalize('en').messageFormatter(
+                    'classdeclaration-validate-duplicatefieldname'
                 );
-            const formatter = Globalize('en').messageFormatter(
-                'classdeclaration-validate-duplicatefieldname'
-            );
-            throw new IllegalModelException(formatter({
-                'class': this.name,
-                'fieldName': duplicateElements[0]
-            }), this.modelFile, this.ast.location);
-        }
+                throw new IllegalModelException(formatter({
+                    'class': this.name,
+                    'fieldName': propertyName
+                }), this.modelFile, this.ast.location);
+            }
+        });
 
         for (let n = 0; n < properties.length; n++) {
             let field = properties[n];

diff --git a/packages/concerto-core/lib/introspect/decorated.js b/packages/concerto-core/lib/introspect/decorated.js
@@ -116,23 +116,20 @@ class Decorated {
             }
 
             // check we don't have this decorator twice
-            const decoratorNames = this.decorators.map(
-                d => d.getName()
-            );
-            const uniqueDecoratorNames = new Set(decoratorNames);
-
-            if (uniqueDecoratorNames.size !== this.decorators.length) {
-                const duplicateElements = decoratorNames
-                    .filter(
-                        (item, index) => decoratorNames.indexOf(item) !== index
+            const uniqueDecoratorNames = new Set();
+            this.decorators.forEach(d => {
+                const decoratorName = d.getName();
+                if(!uniqueDecoratorNames.has(decoratorName)) {
+                    uniqueDecoratorNames.add(decoratorName);
+                } else {
+                    const modelFile = this.getModelFile();
+                    throw new IllegalModelException(
+                        `Duplicate decorator ${decoratorName}`,
+                        modelFile,
+                        this.ast.location,
                     );
-                const modelFile = this.getModelFile();
-                throw new IllegalModelException(
-                    `Duplicate decorator ${duplicateElements[0]}`,
-                    modelFile,
-                    this.ast.location,
-                );
-            }
+                }
+            });
         }
     }
 

diff --git a/packages/concerto-core/lib/introspect/scalardeclaration.js b/packages/concerto-core/lib/introspect/scalardeclaration.js
@@ -17,7 +17,6 @@
 const { MetaModelNamespace } = require('@accordproject/concerto-metamodel');
 
 const Declaration = require('./declaration');
-const IllegalModelException = require('./illegalmodelexception');
 const NumberValidator = require('./numbervalidator');
 const StringValidator = require('./stringvalidator');
 
@@ -106,21 +105,6 @@ class ScalarDeclaration extends Declaration {
      */
     validate() {
         super.validate();
-
-        const declarations = this.getModelFile().getAllDeclarations();
-        const declarationNames = declarations.map(
-            d => d.getFullyQualifiedName()
-        );
-        const uniqueNames = new Set(declarationNames);
-
-        if (uniqueNames.size !== declarations.length) {
-            const duplicateElements = declarationNames.filter(
-                (item, index) => declarationNames.indexOf(item) !== index
-            );
-            throw new IllegalModelException(
-                `Duplicate class name ${duplicateElements[0]}`
-            );
-        }
     }
 
     /**

diff --git a/packages/concerto-core/test/introspect/modelfile.js b/packages/concerto-core/test/introspect/modelfile.js
@@ -206,6 +206,13 @@ describe('ModelFile', () => {
 
     describe('#validate', () => {
 
+        it('should throw when scalar name is duplicted in a modelfile', () => {
+            let asset = introspectUtils.loadModelFile('test/data/parser/scalardeclaration.dupeboolean.cto');
+            (() => {
+                asset.validate();
+            }).should.throw(/Duplicate class/);
+        });
+
         it('should throw when asset name is duplicted in a modelfile', () => {
             let asset = introspectUtils.loadModelFile('test/data/parser/classdeclaration.dupeassetname.cto');
             (() => {

diff --git a/packages/concerto-core/test/introspect/scalardeclaration.js b/packages/concerto-core/test/introspect/scalardeclaration.js
@@ -37,15 +37,6 @@ describe('ScalarDeclaration', () => {
         modelFile = ParserUtil.newModelFile(modelManager, 'namespace com.hyperledger.testing', 'org.acme.cto');
     });
 
-    describe('#validate', () => {
-        it('should throw when scalar name is duplicted in a modelfile', () => {
-            let asset = introspectUtils.loadLastDeclaration('test/data/parser/scalardeclaration.dupeboolean.cto', ScalarDeclaration);
-            (() => {
-                asset.validate();
-            }).should.throw(/Duplicate class/);
-        });
-    });
-
     describe('#accept', () => {
         it('should call the visitor', () => {
             let clz = new ScalarDeclaration(modelFile, {