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...)
>
>
>
>
>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: test.R
URL:
<https://stat.ethz.ch/pipermail/r-devel/attachments/20110128/ce5eee6c/attachment.pl>