Comments for "JpegXmpReader"

» Submit Your Comment
Comments are only accepted during the "Proposal" phase. This proposal is currently in the "Finished" phase.
» Comments
  • Chuck Burgess  [2008-05-21 18:28 UTC]

    Mostly the same comments here as I put on JpegMarkerReader:

    Another light package :)

    Prefix class name with Image_.

    You'll probably also want to look at returning exceptions rather than "false" on failures.

    Also maybe an @link or @see if there's a spec on XMP available online.
  • Philippe Jausions  [2008-05-21 19:56 UTC]

    As Chuck noted, class name should be Image_JpegXmpReader or Image_JPEG_XmpReader.

    On failure return a package exception (for instance Image_JPEG_XmpReader_Exception (extends PEAR_Exception)) instead of FALSE. You can return FALSE if something was not found but is not a critical failure, and could be expected to not be found.

    Please run PHP_CodeSniffer on your code. PEAR uses a 4-space indentation for instance. The first line of docblock cannot be blank, etc...

    You don't need the class constructor if it only calls the parent::__construct()

    Put the declaration of all the members ($xml) in the head of the class, not the very bottom, that's where everybody expect them.

    You'll need to convert the package.xml into version 2.0 (there's a pear command line option for that) If you have examples include them as well.

    If I remember corretly, the first version cannot be 1.0.0+, it has to be something like 0.x.y (Double check PEAR manual though)
  • Daniel O'Connor  [2008-05-22 06:40 UTC]

    Any chance of avoiding:

    return $this->xml->xpath("//dc:$field//rdf:li");

    in favor of restricting to only valid dublincore items?


    Any chance of swapping your $id = "" . sprintf("%c", 0);

    to a constant?

    Otherwise, this looks to be handy!

    (and is Semantic_Web a better home for JpegXmpReader ; as XMP is RDF?)
  • Daniel O'Connor  [2008-05-22 06:42 UTC]

    Not that it probably happens in the wild very often; but how would I extract additional data (RDF) from the image?

    For instance, if the JPEG were authored and contained information about a <foaf:Person> who was depicted...

    Currently having $xml as private prevents me extending / accessing the underlying xml
  • Thomas Boutell  [2008-05-22 13:13 UTC]

    Re: having a class constructor that simply calls the parent class constructor, I saw no other way to ensure the constructor would be documented.

    Re: $xml being private, just call readXmp to return the next XMP XML doc in the JPEG (in most cases: just construct the object and then call readXmp(); but if you had more than one XMP block for some reason you could call more than once).