Added new method for class duplication + changes #17937
Conversation
| RBCondition class >> isClassNamed: className definedIn: aModel [ | ||
|
|
||
| ^ self new | ||
| block: [ | aClassOrTrait | | ||
| aClassOrTrait := aModel classNamed: className. | ||
| aClassOrTrait isNotNil ] | ||
| errorString: [ className , ' is <1?:not > defined' ] | ||
| ^ self new | ||
| block: [ | ||
| | aClassOrTrait | | ||
| aClassOrTrait := aModel classNamed: className. | ||
| aClassOrTrait isNotNil ] | ||
| errorString: className , ' is <1?:not >defined' | ||
| ] |
There was a problem hiding this comment.
Why you couldn't just use this isClassNamed:definedIn:?
Why did you have to duplicate and create isClass:alreadyExistIn:?
| (RBCondition isGlobal: className in: self model) not. | ||
| (RBCondition isSymbol: packageName). | ||
| ((RBCondition withBlock: [ packageName isNotEmpty ]) errorMacro: | ||
| 'Invalid package name') } | ||
| (RBCondition isClass: className alreadyExistIn: self model). | ||
| (RBCondition isValidClassName: className). | ||
| (RBCondition isGlobal: className in: self model) . |
There was a problem hiding this comment.
Could you explain why isGlobal: was not used correctly?
There was a problem hiding this comment.
-Removed block closure for one method because there was an error
The PR description mentioned this, but I cannot find it in the code. Suggestion: try to be more precise when writing these, for example say exactly in which method and because of which error. This can be applied to all points in the PR description. Always try to say WHY you did the change, not WHAT is the change. Because I can easily see WHAT you did, but WHY is the tricky part and that is what I'm reviewing.
|
I understood the point, also I made some mistakes about this PR, im going to make some more changes |
Fixes #17518
-Added method #isClass:alreadyExistIn: to check if the class name is already taken before duplicating
-Removed block closure for one method because there was an error
-Changed the precondition for isGlobal because it was not used correctly