11 Dec
2010
11 Dec
'10
11:31 p.m.
On 12/11/2010 07:20 PM, Christian Henning wrote: > Hi Roland, thanks for spending the time. > >> I think it would be nice to add iterator pairs and boost::range as >> additional devices, though. > I'm thinking of adding input/output iterators for sequential > read/write. Is that what you are thinking of? How would boost:range > fit in? Sorry, never used it. Right, sequential read/write is what I meant. Concerning Boost.Range: I haven't used it myself, yet, but as far as I understand, it is similar to a pair of iterators. I think it is used in quite a few boost libraries, that's why I mentioned them. >>> - 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 > My reason to copy them into detail is to make sure the user isn't > including those by accident. Make sense to you? Would any harm come from including them? I thought it is simply not necessary since they are included by the format specific include files, but if you included them, well, it would still compile but maybe a microsecond slower. > 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... > Maybe a different folder might help? That would be better than the detail folder IMHO > * 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' :-) > tiff_indexed is a property of tiff that is not supported yet. I should > remove it for the time being. How did you find out it? Found it by accident. I wondered where all the tiff things would be used and that is the one I started looking for :-) >>> - What is your evaluation of the documentation? > [...] > All noted. Thanks for writing a review! :-)