Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enh]: Mementos - Only Copy Checked Original #305

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion source/Magritte-GToolkit/MATokenSelectorElement.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ MATokenSelectorElement >> refreshTokens [

| currentValue |
self tokenContainer removeChildren.
currentValue := self object readModelUsing: self relationDescription.
currentValue := self object readUsing: self relationDescription.
currentValue ifNotNil: [ self addTokenFor: currentValue ].
]

Expand Down
4 changes: 3 additions & 1 deletion source/Magritte-Model/MACheckedMemento.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ MACheckedMemento >> original [
{ #category : #actions }
MACheckedMemento >> reset [
super reset.
self setOriginal: self pullRaw
self setOriginal: (self pullRawTransforming: [ :e | e copy ]).

"Implementation note: We copy the field values because checked mementos compare this to the current object to see if it has changed elsewhere. Unless we make a copy each time, this comparison would not be possible for complex objects, because any changes to them from outside will be reflected equally in this `original` dictionary. E.g. if `original at: #person == self model person` and outside someone does `self model person age: 25`, the check above would pass even though it should fail."
]

{ #category : #initialization }
Expand Down
18 changes: 9 additions & 9 deletions source/Magritte-Model/MAMemento.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,20 @@ MAMemento >> pull [

{ #category : #private }
MAMemento >> pullRaw [

^ self pullRawTransforming: [ :e | e ]
]

{ #category : #private }
MAMemento >> pullRawTransforming: aBlock [
| result |
result := Dictionary new.
self magritteDescription do: [ :each |
| value |
| value transformedValue |
value := self model readUsing: each.
result at: each put: value copy "see implementation note" ].
transformedValue := aBlock value: value.
result at: each put: transformedValue ].
^ result

"Implementation note: We copy the field values because checked mementos compare the `original` pull to the current object to see if it has changed elsewhere. Unless we make a copy each time, this comparison would not be possible for complex objects, because any changes to them from outside will be reflected equally in the `original` dictionary. E.g. if `original at: #person == self model person` and outside someone does `self model person age: 25`, the check above would pass even though it should fail."
]

{ #category : #private }
Expand All @@ -109,11 +114,6 @@ MAMemento >> push: aDictionary [
ifTrue: [ self model write: value using: key ] ]
]

{ #category : #private }
MAMemento >> readModelUsing: aDescription [
^ self model readUsing: aDescription
]

{ #category : #private }
MAMemento >> readUsing: aDescription [
^ self subclassResponsibility
Expand Down
7 changes: 0 additions & 7 deletions source/Magritte-Model/Object.extension.st
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,6 @@ Object >> mementoClass [
^ MACheckedMemento
]

{ #category : #'*Magritte-Model' }
Object >> readModelUsing: aDescription [
"This makes Object and MACheckedMemento polymorphic. It is needed e.g. for Magritte forms because inspectable field values should open on the actual object, not the memento's copies. See MAMemento>>#pullRaw for more info"

^ self readUsing: aDescription
]

{ #category : #'*Magritte-Model' }
Object >> readUsing: aDescription [
"This hook is needed so that e.g. mementos and adaptive models can implement their own behavior. All other entry points e.g. MADescription>>#read: should come through here"
Expand Down