[RkWard-devel] New Plug-ins

[SJR]

Am Dienstag, 2. Januar 2007 18:50 schrieb Thomas Friedrichsmeier:
> Hi!
>
> On Sunday 31 December 2006 01:16, SJR wrote:
> > @ Thomas please give me some feed back about my changes and plug-ins
> > regarding quality and so on.
>
> Still haven't looked at everything in detail, but here are a few more
> comments:
> 1) Try to make sure the generated code is always left aligned (unless of
> course the indentation is meaningful). Right now, at many places the code
> is indented with one tab, even though it is top-level statements. Note that
> it does not matter much, whether or not you indent the PHP statements by
> one tab, as is currently done in some plugins, but not in others. However,
> everything between ?> and <? is directly visible to the user, and should be
> left-aligned.

I see. 

>
> 2) When you find yourself using a radio widget with exactly two options,
> it's almost always better to use a checkbox instead. E.g.:
>
> <radio id="narm_skewness" label="Calculate Skewness with Missing Values">
> 	<option value="na.rm = FALSE" label="FALSE"/>
> 	<option value="na.rm = TRUE" label="TRUE"/>
> </radio>
>
> (not that using that wording, you'd also have to exchange TRUE and FALSE)
> would become:
>
> <checkbox id="narm_skewnwss" label="Exclude missing values"
> value="na.rm=TRUE" value_unchecked="na.rm=FALSE"/>
>
> This uses less space, is easier to read, and less complicated all at the
> same time.
>

It was a matter of GUI appearance to me (looked nicer ;) ). I didn't expect 
that it would be better to use a checkbox instead. I'll do so in future. 
Readability and so on are good reasons.

> 3) At some places you use something like
>
> <frame label="The number of the selcted values must be between 3 and
> 5000">...</frame>
>
> to add a comment. I think it would be better not to use a frame, but rather
> add a <text>-element with that note, i.e.:
>
> <text>
> 	Note: The selected vectors must be between 3 and 5000 in length.
> </text>
>
> Using a frame with a label is more suited for giving a caption, than for a
> comment. Also <text> will auto-break, and hence typically look better for
> everything longer than 20-30 characters
>

Yes I know (from a previous thread). This was just done as proof of concept 
and I planned to change it later on actually "converting" to <text> was 
planned when the logic for length is there.

> 4) (Applies to almost all plugins, but I just noted it again while
> reviewing yours) We already have a sort of informal convention to place
> most temporary data in objects called rk.temp*. Probably we should try to
> actually stick to that convention at all places, and make sure we never
> write to any other symbols from plugins (unless the user explicitely asked
> for that, of course). Most importantly, we still have many cases where
> iterators (such as "var" or "i") from for statement are created in the
> workplace. Worse, if the user happens to already have own objects called
> "var" or "i" in the workspace, those are overwritten.
> While it does make for a lot more writing, we should probably rename all
> those to rk.temp.something, too, and e.g. write
>
> rk.temp.i=0;
> for (rk.temp.var in rk.temp.vars) {
> 	rk.temp.i = rk.temp.i+1
> 	...
> }
>
> everywhere.
> A small compensation for the added typing is that we can simplify the
> cleanup () code in most cases to
>
> rm (list=grep ("^rk.temp", ls (), value=TRUE))
>
> I'm willing to go through all existing plugins and fix this there (not
> today, though), but please use this for any further plugins, you create.
>

Yes I'll take care of it. Make a note in the guide for plug-in writing. ;) In 
the past we already had other conventions like trying to use the "R-names" so 
that anybody else can understand what happens.

> 5) Thanks for creating all these new plugins! I just have a hard time
> keeping up with your pace ATM, so responses to all your new plugins and
> suggestions may come in slowly...
>

I can imagine that. Time is the bottleneck for all of us. Therefore I can 
understand quite well that you need some time to catch up. As mentioned 
before, I'll slow down, wait for your changes and fix "my" plug-ins first 
before writing new one for the moment.
As you might have noticed are the most of the new plug-ins derivatives of a 
certain one (e.g., those relying on the nortest package). It was my intention 
to do it this way because I hoped to minimize the workload for you. And as 
you see there are quite a lot of places to be taken care of from my side. 
Thanks for the valuable comments!

> Regards
> Thomas

Stefan