Dear Herve
Thank you once again for taking your time, I appreciate it very much.
I followed your advice and kept only the minimum code, which I believe
is necessary. I have even removed SubClassA, so the inheritance tree is:
BaseClass <- SubClassB <- SubSubClassB1
<- SubClassB <- SubSubClassB2
The source code is shown below and the examples are:
> subsubB1 <- new("SubSubClassB1", filename="MyFileNameB1",
nameB="MyNameB")
> subsubB2 <- new("SubSubClassB2", filename="MyFileNameB2",
nameB="MyNameB")
Although SubSubClassB1 and SubSubClassB2 differ only slightly, the results
for "subsubB1" are correct, while "subsubB2" gives a wrong result, see:
> subsubB2 <- new("SubSubClassB2", filename="MyFileNameB2",
nameB="MyNameB")
> subsubB2
An object of class "SubSubClassB2"
Slot "nameB2":
[1] "MyNameB"
Slot "nameB":
[1] ""
Slot "filename":
[1] "MyFileNameB2"
Only, when adding "nameB2="MyNameB2", I get the correct result, see:
> subsubB2 <- new("SubSubClassB2", filename="MyFileNameB2",
nameB="MyNameB",nameB2="MyNameB2")
> subsubB2
An object of class "SubSubClassB2"
Slot "nameB2":
[1] "MyNameB2"
Slot "nameB":
[1] "MyNameB"
Slot "filename":
[1] "MyFileNameB2"
It is not clear to me why omitting "nameB2" results in the wrong assignemt
of the value for "nameB" to "nameB2"?
When running both examples, the sequence of initialization and validation
method is also completely different.
For "SubSubClassB1" I get the correct and expected sequence of events:
> subsubB1 <- new("SubSubClassB1", filename="MyFileNameB1",
nameB="MyNameB")
------initialize:SubSubClassB1
------initialize:SubClassB
------initialize:BaseClass
------setValidity:BaseClass
------setValidity:SubClassB
------setValidity:SubSubClassB1
In contrast, for "SubSubClassB2" I get the following sequence of events:
> subsubB2 <- new("SubSubClassB2", filename="MyFileNameB2",
nameB="MyNameB")
------initialize:SubSubClassB2
------initialize:SubClassB
------initialize:BaseClass
------setValidity:BaseClass
------initialize:SubClassB
------initialize:BaseClass
------setValidity:BaseClass
------setValidity:SubClassB
------setValidity:SubClassB
------initialize:SubClassB
------initialize:BaseClass
------setValidity:BaseClass
------setValidity:SubClassB
------setValidity:SubSubClassB2
Furthermore, the slot "filename" is first initialized correctly to
"filename=MyFileNameB2", but then it is twice initialized incorrectly
to "filename=ERROR_FileName", indicating that it may not have been
initialized at all. I do not understand this behavior, why is this so?
I really hope that this time the code below is acceptable to you.
Thank you for your help.
Best regards
Christian
# - - - - - - - - - - - - - - - - BEGIN - - - - - - - - - - - - - - - -
- - - -
setClass("BaseClass",
representation(filename = "character", "VIRTUAL"),
prototype(filename = "DefaultFileName")
)
setClass("SubClassB",
representation(nameB = "character"),
contains=c("BaseClass"),
prototype(nameB = "NameB")
)
setClass("SubSubClassB1",
contains=c("SubClassB")
)
setClass("SubSubClassB2",
representation(nameB2 = "character"),
contains=c("SubClassB"),
prototype(nameB2 = "NameB2")
)
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
setMethod("initialize", "BaseClass",
function(.Object, filename="", ...) {
cat("------initialize:BaseClass------\n")
cat("BaseClass:init:class(.Object) = ", class(.Object), "\n", sep="")
if (filename == "" || nchar(filename) == 0) filename <-
"ERROR_FileName"
cat("BaseClass:init:filename = ", filename, "\n", sep="")
.Object <- callNextMethod(.Object, filename=filename, ...)
[EMAIL PROTECTED] <- filename
.Object
}
)
setValidity("BaseClass",
function(object) {
cat("------setValidity:BaseClass------\n")
cat("BaseClass:val:class(.Object) = ", class(object), "\n", sep="")
msg <- NULL
if (!(is.character([EMAIL PROTECTED])))
msg <- cat("missing filename\n")
cat("BaseClass:val:filename = ",[EMAIL PROTECTED], "\n", sep="")
if (is.null(msg)) TRUE else msg
}
)
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
setMethod("initialize", "SubClassB",
function(.Object, nameB="", ...) {
cat("------initialize:SubClassB------\n")
cat("SubClassB:init:class(.Object) = ", class(.Object), "\n", sep="")
.Object <- callNextMethod(.Object, nameB=nameB, ...)
[EMAIL PROTECTED] = nameB
.Object
}
)
setValidity("SubClassB",
function(object) {
cat("------setValidity:SubClassB------\n")
cat("SubClassB:val:class(object) = ", class(object), "\n", sep="")
msg <- NULL
if (!(is.character([EMAIL PROTECTED]) && length([EMAIL PROTECTED]) > 0))
msg <- cat("missing nameB\n")
cat("SubClassB:val:nameB = ",[EMAIL PROTECTED], "\n", sep="")
if (is.null(msg)) TRUE else msg
}
)
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
setMethod("initialize", "SubSubClassB1",
function(.Object, ...) {
cat("------initialize:SubSubClassB1------\n")
cat("SubSubClassB1:init:class(.Object) = ", class(.Object), "\n",
sep="")
.Object <- callNextMethod(.Object, ...)
.Object
}
)
setValidity("SubSubClassB1",
function(object) {
cat("------setValidity:SubSubClassB1------\n")
cat("SubSubClassB1:val:class(object) = ", class(object), "\n", sep="")
return(TRUE)
}
)
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
setMethod("initialize", "SubSubClassB2",
function(.Object, nameB2="MyNameB2", ...) {
cat("------initialize:SubSubClassB2------\n")
cat("SubSubClassB2:init:class(.Object) = ", class(.Object), "\n",
sep="")
if (nameB2 == "") nameB2 <- "DefaultNameB2";
cat("SubSubClassB2:init:nameB2 = ", nameB2, "\n", sep="")
.Object <- callNextMethod(.Object, nameB2=nameB2, ...)
[EMAIL PROTECTED] <- nameB2
.Object
}
)
setValidity("SubSubClassB2",
function(object) {
cat("------setValidity:SubSubClassB2------\n")
cat("SubSubClassB2:val:class(object) = ", class(object), "\n", sep="")
msg <- NULL;
if (!(is.character([EMAIL PROTECTED]) && length([EMAIL PROTECTED]) > 0))
msg <- cat("missing nameB2\n")
cat("SubClassB:val:nameB2 = ",[EMAIL PROTECTED], "\n", sep="")
if (is.null(msg)) TRUE else msg
}
)
# - - - - - - - - - - - - - - - END - - - - - - - - - - - - - - - - -
Herve Pages wrote:
> Hi Christian,
>
> cstrato wrote:
>
>> Dear Herve
>>
>> Thank you for your helpful comments, and I especially appreciate that
>> you tried to run my package. I will try to answer each point separately.
>>
>> Herve Pages wrote:
>>
>>> Hi Christian,
>>>
>>> I can only give you a few reasons why IMO it is very unlikely that
>>> anybody
>>> will be able to help you on this, with the current form of your post.
>>>
>>> 1) Unless you have a really good reason to do so, don't attach a package
>>> to your post. Do your best to provide a few lines of code that anybody
>>> can easily copy and paste into their session.
>>>
>>>
>> Sorrowly, sometimes, a few lines of code are not sufficient to show
>> the problem. Furthermore, most of the time there are complaints that
>> people do not provide enough information, an issue I wanted to avoid.
>>
>
> The code you provide below is still too long and overcluttered with stuff
> that is
> probably unrelated with the issue you want to discuss. Your class definitions
> still have slots that we don't care about. Basically if you want to
> discuss an S4 issue, you should get rid of all this file system related stuff
> (the 'dirfile', 'filedir', 'filename' slots, the 'pathFile' function, the
> dozens
> of calls to 'basename', 'dirname', 'getwd', 'file.dir' etc...)
>
> Also your code is dirty and hard to read. Take this for example:
>
> "initialize.BaseClass" <-
> function(.Object, filename=character(), filedir=as.character(getwd()), ...)
> {
> print("------initialize:BaseClass------")
> print(paste("BaseClass:init:class(.Object) = ", class(.Object)))
>
> # .Object <- callNextMethod(.Object, ...);
>
> dirfile <- pathFile(filename, filedir);
> print(paste("BaseClass:init:dirfile = ", dirfile))
>
> .Object <- callNextMethod(.Object, filename=filename, filedir=filedir,
> ...);
>
> [EMAIL PROTECTED] <- filename;
> [EMAIL PROTECTED] <- filedir;
> .Object;
> }#initialize.BaseClass
>
> setMethod("initialize", "BaseClass", initialize.BaseClass);
>
> o It's not properly indented.
> o Why those empty lines in the middle of such a short function?
> o Why those semi-columns at the end of lines?
> o Why put the implementation of the initialize method into a separate
> function?
> o Why use this construct 'print(paste())' instead of just 'cat()'?
> o Why leave the first call to callNextMethod() commented?
> o What do you do with 'dirfile', except that you print it? Do you need this
> for
> the purpose of the S4 discussion?
>
> What about adopting this style:
>
> setMethod("initialize", "BaseClass",
> function(.Object, filename=character(),
> filedir=as.character(getwd()), ...)
> {
> cat("------initialize:BaseClass------\n")
> cat("BaseClass:init:class(.Object) = ", class(.Object))
> .Object <- callNextMethod(.Object, filename=filename,
> filedir=filedir, ...)
> [EMAIL PROTECTED] <- filename
> [EMAIL PROTECTED] <- filedir
> .Object
> }
> )
>
> That's for the style. Now let's talk about the semantic. Here is the
> definition of
> your BaseClass class:
>
> setClass("BaseClass",
> representation(
> "VIRTUAL",
> filename="character",
> filedir="character"
> ),
> prototype(
> filename = character(),
> filedir = as.character(getwd())
> )
> )
>
> (Note that this definition is what _I_ get after I cleaned it and indented it
> which
> is something that _you_ are expected to do.)
>
> o Initializing the 'filename' slot to character() is useless since this is
> the default.
> o Wrapping getwd() inside as.character() is useless since getwd() returns a
> character vector.
> o BUT MOST IMPORTANTLY THAN ANYTHING ELSE: given the fact that you've
> provided a prototype
> for class BaseClass, your initialize method is useless since it does
> _nothing_ more
> than what the default initialize method does! If you want to define your
> own "initialize"
> method for the only purpose of printing messages, then you could do it in a
> much simpler
> way:
>
> setMethod("initialize", "BaseClass",
> function(.Object, ...)
> {
> cat("------initialize:BaseClass------\n")
> cat("BaseClass:init:class(.Object) = ", class(.Object), "\n",
> sep="")
> callNextMethod()
> }
> )
>
> But as I said initially, the file system related stuff is useless to
> illustrate the
> S4 issue you want to show us so why didn't you just provide:
>
> setClass("BaseClass",
> representation("VIRTUAL", slot1="character"),
> prototype(slot1="aaa")
> )
>
> setMethod("initialize", "BaseClass",
> function(.Object, ...)
> {
> cat("------initialize:BaseClass------\n")
> cat("BaseClass:init:class(.Object) = ", class(.Object), "\n",
> sep="")
> callNextMethod()
> }
> )
>
> See? I ended up by replacing 25 lines of your hard-to-read/confusing code by
> 12
> lines of clean code containing only the STRICTLY NECESSARY stuff for an S4
> discussion.
> That's what you should have done instead of sending us 150 lines of code.
>
> Sorrowly people will not try to read your code unless it's obvious that _you_
> did your
> best to provide the cleanest/simplest possible code that illustrates the
> issue you want
> to discuss.
>
> Also maybe you could try to read a little bit more about S4 and
> initialization mechanisms.
>
> Cheers,
> H.
>
>
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
