[PEPr] Comment on HTML::HTML_Flash

Ian Eure (http://pear.php.net/user/ieure) has commented on the proposal for HTML::HTML_Flash. Comment: - This should be named HTML_FlashEmbed, to make it clear that it's not used for generating actual flash files. - The debug flag appears to control markup formatting. This makes no sense. - _addLn() is inscrutable and inefficient. A simple: return $this->_getLineEnd() . str_repeat($this->_getTabs($id)); does everything that function does. Alternately, HTML_Common::_getTabs() should work (it does the same thing,) if you set $_tabOffset. - Why is the 'line end' put at the beginning of the string in _addLn()? - Why are you generating attributes by hand in toHtml() when HTML_Common::_getAttrString() exists already? - There's a fundamental disconnect in your code vs. HTML_Common. HTML_Common is intended to represent a single HTML element, while your code handles several, and has to jump through hoops to do so. You should find a cleaner approach. - display() is unnecessary. - Most private member vars should probably be public; - Includes PEAR.php, which is never used. - toHtml() does a long check on the presence of individual attributes, when an array_merge() is clearer, and probably more efficient. Alternately, set your default attrs in the constructor (or the $_attributes var) and don't worry about it in toHtml(). - CS needs cleanup: + Missing spaces around operators, e.g. , . = == etc (same for function definitions with defaults,) missing spaces around parens around control structures in some places, indentation is screwed up in some places as well. - phpDocumentor blocks are incomplete, contain typos, and aren't useful. '@param int' means very little, there should be a var name and description of what it does as well. 'Attribute' is misspelled as 'Attribut,' and 'debugging' is misspelled as 'bebugging.' - There should be at least four modes: valid html/xhtml, and invalid html/xhtml. The "normal" Flash markup is invalid, but can be made completely valid and (mostly) functional. See http://www.alistapart.com/articles/flashsatay/ Good idea, but the implementation definitely needs work. Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=277 -- Sent by PEPr, the automatic proposal system at http://pear.php.net