
11 Dec
2010
11 Dec
'10
3:35 p.m.
Hi, here is a short review of GIL new IO (I missed the documentation of the toolbox, which is why I concentrated on IO). > - What is your evaluation of the design? Quite elegant and intuitive. I think it would be nice to add iterator pairs and boost::range as additional devices, though. > - What is your evaluation of the implementation? All the code I've looked at so far looks well organized and well readable. Some remarks: * Why are the following files in the detail folder? io_new/detail/read_and_convert_image.hpp io_new/detail/read_and_convert_view.hpp io_new/detail/read_image.hpp io_new/detail/read_image_info.hpp io_new/detail/read_view.hpp io_new/detail/write_view.hpp They define functions of the public interface like read_image(). There is no need to include them directly, but still, when I want to look at documented functions, it seems weird to go into the detail folder... * There seem to be some remnants from earlier versions. For instance: struct tiff_indexed is defined in io_new/tiff_tags.hpp, but never used anywhere else (or did I miss something?) io_new/detail/typedefs.hpp ends with } // namespace details <-- notice the 's' :-) >- What is your evaluation of the documentation? As stated above, I missed the documentation on the toolbox. The documentation is well readable and easy to understand. There seem to be broken links: * "section Extending GIL::IO with new Formats" links to index.html * "Please see section Supported Image Formats." links to index.html * "sections _Supported_Formats_ for more details." links to / It would be good to have code samples for read_and_convert_image, read_view and read_and_convert_view in the tutorial. The "Using IO" section seems to assume that they are already documented: "As the Tutorial demonstrated there are a few ways to read images." Additionally, it would be great to have a reference section. :-) I stumbled over a few typos and grammatically unusual sentences (IMHO): * s/supoorted/supported/ * "Before we can read or write image one thing." I know what that means in German, but does this make sense in English? * "If the user doesn't know what he is dealing with it can use read_and_convert_image()." s/it/he/ I think it would make much sense to give the text to an English native speaker. > - What is your evaluation of the potential usefulness of the extensions? Very useful! > - Did you try to use the extensions? With what compiler? Did you have any problems? I did not actually build anything. Sorry :-( But I am looking forward to doing so! > - How much effort did you put into your evaluation? A glance? A quick > - reading? In-depth study? About half a day of reading. > - Are you knowledgeable about the problem domain? I have used several other libraries over the last 15 years, but I never delved into the innards of image file formats... > - Do you think the extensions should be accepted as a part of Boost.GIL library? Yes, with a few wishes (see above): - documentation: - typos and grammar - add reference section - add a few more code samples in the tutorial - file structure - move parts of the public interface into the non-detail area - interface: - add support for iterators and ranges Again, I did not evaluate the toolbox part. Thanks writing this extension! Roland