Hi Marc,
Sort of one out of two, but very helpful.
On 1/28/11 11:01 AM, Marc Carpentier wrote:
Dear r-devel-list, dear John Chambers,
I'm trying to learn OOP-possibilities in R and I was going through the
documentation 'ReferenceClasses {methods}' (great work, by the way...).
Reading associated Examples, something bothers me : it seems to me that there
are errors in 'edit' and 'undo' methods. I think that :
- 'undo' should update 'edits' field with :
length(edits)<<- length(edits) - 1 #(and not - 2)
Nope. There are actually two logical choices here, but that is not
either of them.
Notice that the line before that is:
edit(prev[[1]], prev[[2]], prev[[3]])
which invokes the $edit() method to effect the undo, _and_ which adds
that edit to the $edits list.
One could just leave things that way, but we decide to hide our undo
from Wikileaks by removing both the edits.
- and for coherence, 'edit' should store modifications in an 'append'-style :
edits<<- c(edits,list(backup)) #as opposed to c(list(backup),edits)
Well, that's a bit debatable, but it does expose a bug, for sure. The
current order might be acceptable, but $undo() is then removing the
wrong end of the $edits list, as would have been obvious if the example
had done two edits and then removed one. In the current version the
first backup is the most recent edit (and indeed is used to reset the
data), but then the wrong elements of $edits are removed.
Given that, I agree that the opposite order of keeping the backup list
is better. Less copying, for one thing.
Attached is a corrected and slightly expanded version of that part of
the example. Anyone is encouraged to try it out; if no further problems
arise, I'll commit its essentials.
I hope I'm not wrong and this hasn't been previously reported (I didn't find
anything about it)
Best regards.
PS: I first posted this message on help-list. David Winsemius suggested me
devel-list would probably be more appropriate and was right about that. Sorry if
you read it again.
And indeed r-devel was the right place. Thanks
John
PPS: please associate my address when responding because I didn't subscribe to
r-devel-list (I'm still far from being able to follow all its discussions...)
mEditor <- setRefClass("matrixEditor",
fields = list( data = "matrix",
edits = "list"),
methods = list(
edit = function(i, j, value) {
## the following string documents the edit method
'Replaces the range [i, j] of the
object by value.
'
backup <-
list(i, j, data[i,j])
data[i,j] <<- value
edits <<- c(edits, list(backup))
invisible(value)
},
undo = function() {
'Undoes the last edit() operation
and update the edits field accordingly.
'
prev <- edits
if(length(prev)) prev <- prev[[length(prev)]]
else stop("No more edits to undo")
edit(prev[[1]], prev[[2]], prev[[3]])
## trim the edits list
length(edits) <<- length(edits) - 2
invisible(prev)
}
))
xMat <- matrix(1:12,4,3)
xx <- mEditor$new(data = xMat)
xx$edit(2, 2, 0)
xx$data
xx$edit(1,3, -1)
xx$data
xx$edits
xx$undo()
xx$edits
xx$data
xx$undo()
xx$data
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
