Refactoring and efficiency problem solved

pharo-spec · Oct 23, 2023 · 4adf107 · 4adf107
1 parent b8320c5
commit 4adf107
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 62 deletions.
diff --git a/src/NewTools-Scopes-Browser/ScopeScopeNode.class.st b/src/NewTools-Scopes-Browser/ScopeScopeNode.class.st
@@ -13,24 +13,24 @@ Class {
 ScopeScopeNode >> children [
 
 	| className class package orphanClasses fullPackages envPackages envClasses chldrn packageEnvironments classesEnvironments |
-
+
+	children ifNotNil: [ ^ children ].
+
 	self value isCompositeEnvironment ifTrue: [
 		chldrn := Set new.
 		envPackages := self value packages.
-		envClasses := self value classes.
-		"classes whose package hasn't all of its classes selected"
-		orphanClasses := envClasses select: [ :each |
-			                 | siblings |
-			                 siblings := each package definedClasses.
-			                 siblings anySatisfy: [ :sibling |
-				                 envClasses noneSatisfy: [ :selectedClass |
-					                 selectedClass = sibling ] ] ].
+		envClasses := self value classes reject: [:c | c isMeta ].
 
 		"packages whose classes were all selected"
-		fullPackages := envPackages select: [ :each |
-			                each definedClasses noneSatisfy: [ :each1 |
-				                orphanClasses anySatisfy: [ :orphan |
-					                orphan = each1 ] ] ].
+		fullPackages := ScopesManager
+			                fullPackagesIn: envPackages
+			                having: envClasses.
+		"classes whose package hasn't all of its classes selected"
+
+		orphanClasses := ScopesManager
+			                 orphanClassesIn: envClasses
+			                 havingPackages: envPackages
+			                 fullPackages: fullPackages.
 
 		packageEnvironments := fullPackages collect: [ :thePackage |
 			                       ScopePackageNode on: thePackage ].
@@ -40,23 +40,26 @@ ScopeScopeNode >> children [
 				                       ifFalse: [ ScopeClassNode on: theClass ] ].
 		packageEnvironments do: [ :env | chldrn add: env ].
 		classesEnvironments do: [ :env | chldrn add: env ].
-		^ chldrn ].
+		^ children := chldrn ].
+
+	^ children := self value basisObjects collect: [ :stringOrPackage |
+		            stringOrPackage isString
+			            ifTrue: [
+				            className := stringOrPackage.
+				            class := self systemDictionary
+					                     at: className
+					                     ifAbsent: [ nil ].
+				            class isTrait
+					            ifTrue: [ ScopeTraitNode on: class ]
+					            ifFalse: [ ScopeClassNode on: class ] ]
+			            ifFalse: [
+				            stringOrPackage isClassOrTrait
+					            ifTrue: [ ScopeClassNode on: stringOrPackage ]
+					            ifFalse: [
+						            self assert: (stringOrPackage isKindOf: RPackage).
+						            package := stringOrPackage.
+						            ScopePackageNode on: package ] ] ].
 
-	^ self value basisObjects collect: [ :stringOrPackage |
-		  stringOrPackage isString
-			  ifTrue: [
-				  className := stringOrPackage.
-				  class := self systemDictionary at: className ifAbsent: [ nil ].
-				  class isTrait
-					  ifTrue: [ ScopeTraitNode on: class ]
-					  ifFalse: [ ScopeClassNode on: class ] ]
-			  ifFalse: [
-				  stringOrPackage isClassOrTrait
-					  ifTrue: [ ScopeClassNode on: stringOrPackage ]
-					  ifFalse: [
-						  self assert: (stringOrPackage isKindOf: RPackage).
-						  package := stringOrPackage.
-						  ScopePackageNode on: package ] ] ]
 ]
 
 { #category : 'testing' }

diff --git a/src/NewTools-Scopes-Browser/ScopesPresenter.class.st b/src/NewTools-Scopes-Browser/ScopesPresenter.class.st
@@ -140,13 +140,16 @@ ScopesPresenter >> browseSelectedItem [
 ScopesPresenter >> deleteSelectedScope [
 
 	| item selectedPath |
-	selectedPath := scopesTree selection selectedPath at: 1.
-	selectedPath == 1 ifTrue: [
-		self inform: 'You can''t delete this scope'.
-		^ self ].
 	item := scopesTree selectedItem.
-	(item isNil or: [ item isScopeNode not ]) ifTrue: [
-		self inform: 'select a scope'.
+	(item isNil or: [
+		 item isScopeNode not or: [
+			 (selectedPath := scopesTree selection selectedPath) isEmpty ] ])
+		ifTrue: [
+			self inform: 'select a scope'.
+			^ self ].
+
+	(selectedPath at: 1) == 1 ifTrue: [
+		self inform: 'You can''t delete this scope'.
 		^ self ].
 
 	model removeScope: item value.
@@ -159,10 +162,19 @@ ScopesPresenter >> deleteSelectedScope [
 { #category : 'menu commands' }
 ScopesPresenter >> editSelectedScope [
 
+	| selectedPath |
 	(scopesTree selectedItem isNil or: [
-		 scopesTree selectedItem isScopeNode not ])
-		ifTrue: [ self inform: 'select a scope' ]
-		ifFalse: [ self newScopeEditor open ]
+		 scopesTree selectedItem isScopeNode not or: [
+			 (selectedPath := scopesTree selection selectedPath) isEmpty ] ])
+		ifTrue: [
+			self inform: 'select a scope'.
+			^ self ].
+
+	(selectedPath at: 1) == 1 ifTrue: [
+		self inform: 'You can''t edit this scope'.
+		^ self ].
+
+	self newScopeEditor open
 ]
 
 { #category : 'initialization' }
@@ -188,7 +200,7 @@ ScopesPresenter >> initializeScopesTree [
 	^ scopesTree
 		  hideColumnHeaders;
 		  addColumn: self nodesListColumn;
-		  children: [ :each | each children sorted ];
+		  children: [ :each | each children asOrderedCollection ];
 		  roots: self scopeNodes;
 		  expandRoots;
 		  collapsePath: #( 1 );
@@ -342,7 +354,6 @@ ScopesPresenter >> scopeListColumn [
 
 { #category : 'initialization' }
 ScopesPresenter >> scopeNodes [
-
 	^ { (ScopeScopeNode on: model defaultScope) } asOrderedCollection
 		  addAll: (model scopes collect: [ :s | ScopeScopeNode on: s ])
 				  asOrderedCollection sorted;

diff --git a/src/NewTools-Scopes/ScopesManager.class.st b/src/NewTools-Scopes/ScopesManager.class.st
@@ -7,7 +7,8 @@ Class {
 	#instVars : [
 		'scopes',
 		'environment',
-		'announcer'
+		'announcer',
+		'defaultScope'
 	],
 	#classVars : [
 		'singleInstance'
@@ -28,11 +29,31 @@ ScopesManager class >> availableScopes [
 	^ self scopes
 ]
 
+{ #category : 'private' }
+ScopesManager class >> fullPackagesIn: packages having: classes [
+	"packages whose classes were all selected"
+
+	^ packages select: [ :package |
+		  | packCheckedClasses |
+		  packCheckedClasses := classes select: [ :class |
+			                        class package == package ].
+		  package definedClasses size == packCheckedClasses size ]
+]
+
 { #category : 'accessing' }
 ScopesManager class >> invalidNames [
 	^ self scopes collect: [ :e | e label ]
 ]
 
+{ #category : 'private' }
+ScopesManager class >> orphanClassesIn: classes havingPackages: packages fullPackages: fullPackages [
+
+	| classesInFullPackages |
+	classesInFullPackages := fullPackages flatCollect: [ :package |
+		                         package definedClasses ].
+	^ classes reject: [ :class | classesInFullPackages includes: class ]
+]
+
 { #category : 'accessing' }
 ScopesManager class >> removeScope: aScope [
 	self singleInstance removeScope: aScope
@@ -80,8 +101,10 @@ ScopesManager >> announcer [
 { #category : 'api' }
 ScopesManager >> defaultScope [
 
-	^ (self scopeClass packages: self allPackagesInEnvironment) label:
-		  self defaultScopeLabel
+	^ defaultScope ifNil: [
+		  defaultScope := (self scopeClass packages:
+			                   self allPackagesInEnvironment) label:
+			                  self defaultScopeLabel ]
 ]
 
 { #category : 'accessing' }
@@ -102,6 +125,11 @@ ScopesManager >> environment: aSmalltalkImage [
 	environment := aSmalltalkImage
 ]
 
+{ #category : 'private' }
+ScopesManager >> fullPackagesIn: packages having: classes [ 
+	^ self class fullPackagesIn: packages having: classes 
+]
+
 { #category : 'initialization' }
 ScopesManager >> initialize [
 
@@ -113,37 +141,32 @@ ScopesManager >> initialize [
 ScopesManager >> newScopeFrom: aSet [
 
 	| packageScope classScope classes packages orphanClasses fullPackages compositeScope |
-
 	classes := aSet select: [ :each | each isClassOrTrait ].
 	packages := aSet select: [ :each | each isKindOf: RPackage ].
 
-	"classes whose package hasn't all of its classes selected"
-	orphanClasses := classes select: [ :class |
-		                 | siblings |
-		                 siblings := class package definedClasses.
-		                 siblings anySatisfy: [ :sibling |
-			                 classes noneSatisfy: [ :selectedClass |
-				                 selectedClass = sibling ] ] ].
-
 	"packages whose classes were all selected"
-	fullPackages := packages select: [ :each |
-		                each definedClasses noneSatisfy: [ :class |
-			                orphanClasses anySatisfy: [ :orphan |
-				                orphan = class ] ] ].
-
+	fullPackages := self class fullPackagesIn: packages having: classes.
+	"classes whose package hasn't all of its classes selected"
+	orphanClasses := self class
+		                 orphanClassesIn: classes
+		                 havingPackages: packages
+		                 fullPackages: fullPackages.
+
 	classScope := RBClassEnvironment classes: orphanClasses.
 	packageScope := RBPackageEnvironment packages: fullPackages.
-
-	self flag: 'hacer recursiva la construccion del composite scope'.
-	(classScope isNotEmpty and: [ packageScope isNotEmpty ]) ifTrue: [ 
+
+	(classScope isNotEmpty and: [ packageScope isNotEmpty ]) ifTrue: [
 		compositeScope := packageScope | classScope ].
 
 	compositeScope ifNotNil: [ ^ compositeScope ].
 	packageScope isEmpty ifFalse: [ ^ packageScope ].
 	classScope isEmpty ifFalse: [ ^ classScope ].
-	self error: 'Impossible to create scope from nodes'.
-	
+	self error: 'Impossible to create scope from nodes'
+]
 
+{ #category : 'private' }
+ScopesManager >> orphanClassesIn: classes havingPackages: packages fullPackages: fullPackages [ 
+	^ self class orphanClassesIn: classes havingPackages: packages fullPackages: fullPackages 
 ]
 
 { #category : 'removing' }
@@ -158,6 +181,7 @@ ScopesManager >> reset [
 
 	environment := nil.
 	self announcer subscriptions reset.
+	defaultScope := nil.
 	scopes := #(  )
 ]