Re: [PEPr] Images::Imagick proposal, Naming Issues

> Please re-review the proposal: > http://pear.php.net/pepr/pepr-proposal-show.php?id=64 > > I also hope to get some input on "Image_Magick_Conjure" by Florent Monnier. > He doesn't use a PEAR-Imagick-Api right now and it would be nice to get > together somehow. =========== I am not sure 'Image_Imagick' is the better name, 'Image_Magick' would be enough and better I think. Pecl 'imagick' is an abbr for 'Image-Magick'. As this class will go in the Image directory perhaps there is no need to add another redondant 'I' meaning 'Image' twice? Even if it is through pecl-imagick wraper, Image_Magick wraps to Image-Magick at the final point. pecl->imagick is just a link in this chain (I'll come back on this point below). Moreover it would be more logical and readable: require_once 'Image/Magick.php'; $im = &new Image_Magick; $im->something(...); Have a look at this Perl sample: $im = new Image::Magick; $im->Read("image.orig.png"); $im->Resize(width => 600, height => 450); $im->Write(filename => "image.new.png"); The name is Image-Magick not Image-Imagick. If leaving back the image 'I' abbr part of the name for the php filename, consider the pdf version of the exhaustive Image-Magick documentation: " magick.pdf " :) http://imagemagick.sourceforge.net/docs/magick.pdf Even in the desing of your class, I think INHO that you should keep in mind that your class should wrap to IM, and not to pecl-imagick. Who this class is designed to? Only php users or also for every-day IM users, and users from the IM community? If this class is also intended to be used by IM users, it should respect the IM standard names of the operations, a lot of actions have been hardly customized with prefix and suffix, and even been renamed. The magick.pdf doc contains the doc for the 4 majors intefaces of IM: - the command line - the C api - the C++ api - the Perl api (There are other apis like scheme, py, php, but all those are very alpha.) There are small differences, but it is quite uniform and standard. Just consider respecting the standard IM command names like respecting Pear coding standard! However to fit for users that are not IM users and that don't know IM, names like 'doubleSizeImage' are indeed more understandable than 'magnify'. Renaming magnify to doubleSizeImage, and minify to halfSizeImage is perhaps quite safe IMHO for the far more tricky names, though if you expect that IM users will find the operations they are used to, perhaps aliases should be made? If you do not want to keep IM standard operations naming, and if you do not want to provide alias for those, you should perhaps give correspondances in the documentation for those users. A solution could be to use aliases, and the main function should be the standard IM ones' named. (If this class would only have to fit to me as an every day IM and PHP user, or if I would have wrote such this underlaying class for conjure, I would just have kept all the standard IM names, since the advantage for this would have been to let users have unified support from the IM community, where on the IM mailing-list there are all kind of users, Perl, C, php-imagick, etc.. and everybody can understand each other's tips through the common IM language) In case the Pear community would make the choice to rename the IM language, perhaps we should take 5 min to consider each renamed operation. 'setContrast' as a loop around pecl::contrast could good for non-IM users. Nevertheless adding the 'set' prefix or not should be considered seriously. Perhaps a mail to the IM user mailing-list, would be a good idea to see what do users think about this naming issue. == Details about functions name: I find the naming of the functions is quite weird. I am not sure 'Filter' and 'add' are necessary in the function names. But if you choose to keep these, it would be a good idea I think not to put the reccurent part 'Filter' at the end to keep scripts readable: $IM->addFilterSwirl(30); $IM->addFilterEmboss(3, 4); $IM->addFilterShade(60, 30, 8); Why do you consider some imagick_*() functions as filter and not the other? edgeImage(), 'imagick_edge()' isn't it a filter? And imagick_medianfilter() ? Why not addMedianFilterFilter() ? :p :) Though the "Filter" classification you can find in the pecl->im source code, I am not sure this exists in IM. http://chora.php.net/co.php/pecl/imagick/imagick.c?php=8903330c4c8d8c7ca06c193f821057cf&r=1.52 (@todo check this in IM src:) Moreover if you want a prefix, I am not sure at all this one 'addFilter' would be the good one. And why sometimes you add 'Image', all the operations act on the image. embossImage(), why not addFilterEmboss(3, 4) ? I think there is indeed perhaps a need for a prefix. If so here are some ideas: apply, make, process, ... I have search in my fench->english dictonary, perhaps the apply prefix would be good? It seems to fit with paint vocabulary. $IM->applySwirl(30); $IM->applyEmboss(3, 4); $IM->applyShade(60, 30, 8); Adding a prefix would have the advantage to prevent the 'Implode' function to colide with the PHP one. (@todo check if collisions are possible with a class method) Personnaly I would prefer no prefix, but if pear community choose to put one, I would prefer only one unified prefix, for all the pear::IM functions, rather than classified function prefix which will be very tricky to make the decision for some functions that are not clearly definable and classificable. It is in Perl that the IM module is the far most advanced one. (I'm only considering the high level languages to say that.) I don't say that the Pear one should copy-past all the Perl api, but getting inspiration from it would be greatly beneficial, since there is a big amount of experience in it! -- Best Regards Florent