Re: HTML QuickForm elments for HTMLArea (xinha), FCKeditor and jscalendar

On 5/23/06, Jan Wagner <wagner@netsols.de> wrote:

Hi everyone, as promised a long time ago, I finally managed to put the mentioned elements into PEAR packages. These and examples can be found here: http://www.netsols.de/pear/htmlarea http://www.netsols.de/pear/fckeditor http://www.netsols.de/pear/jscalendar As these packages are really simple, I don't know if they are worth to be separate packages. The QuickForm authors mentioned once, that they would prefer it that way, but I'll wait for others to speak up.

Comments on the code: "$name".....why? As Phillipe said, don't use sprintf, especially for simple string replacement. printf is *slow*. It's also a pain to read. http://pear.reversefold.com/strings/ As shown in the above link, this is also slow: ",{$button['context']}" I would prefer: ','.$button['context'] especially when you're concatenating a single char. I also like concatenation better than " because it is syntax highlighted correctly in all editors is just much easier to eyeball. There is some inconsistent quote usage, " where you could be using '. It is generally better practice to build a string as a literal then run htmlentities() on it before you put it into HTML. If you'd rather hard-code your &amp; (no problems with this) then make sure to run htmlentities() on any variables as it's possible they could have entities that need to be escaped (such as BasePath). Between the elements you're inconsistently using member vars vs. options. For example, $this->BasePath in fckeditor and $this->_options['basePath'] in jscalendar. I would prefer using the options array over member variables. If you're going to propose these to PEAR you also should look at your variable naming. Member variables should be named with studlyCaps, not AllCaps. There also seems to be some overlap in JS functions, such ass array2js. Perhaps you could make a base class for these elements which holds these functions? -- Justin Patrin