[PEPr] Comment on Images::JpegMarkerReader

Philippe Jausions (http://pear.php.net/user/jausions) has commented on the proposal for Images::JpegMarkerReader. Comment: - Package name (and class) should be Image_JpegMarkerReader or Image_JPEG_MarkerReader. - As Chuck noted, throw exceptions on failures (your package's exception class should extend PEAR_Exception); for instance on corrupted files. FALSE is appropriate if this is not an error (i.e. it could be expected to not find something.) - Look into implementing SPL's Iterator (or IteratorAggregate) interfaces, so it would be very easy to do a foreach() on all the markers. - Run PHP_CodeSniffer on your class to help correct PEAR coding standard issues (4-space indent, all private members name should start with underscore "_", etc...) - The "filename" member is not declared - "$this->in" is atypical for a file pointer name. Not a deal breaker by any measure, since it's private, but $this->_fp or $this->_file are usually more common. - You may consider returning NULL instead of FALSE for the standalone markers in skipMarkersIfNot() Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=559 -- Sent by PEPr, the automatic proposal system at http://pear.php.net