[Liblas-devel] Overriding a Reader's header

[Mateusz Loskot]

Howard Butler wrote:
> 
> On Oct 6, 2009, at 2:13 PM, Mateusz Loskot wrote:
> 
>> Howard Butler wrote:
>>> I propose that we provide a
>>> 
>>> LASReader(std::istream& ifs, LASHeader& header)
>>> 
>>> constructor, and if this constructor is used, this header is used
>>>  to describe the data except for the following condition... we 
>>> respect the dataoffset as it exists in the file (why it isn't 
>>> const).  This will allow a user to override things like the 
>>> coordinate system, scales, dates, etc but still allow the file to
>>>  be read.
>>> 
>>> Make sense?
>> 
>> Hobu,
>> 
>> Sure, it looks perfectly OK to me, except I would pass header by 
>> const:
>> 
>> LASReader(std::istream& ifs, LASHeader const& header)
> 
> We can't pass it in const because the caller must be ready to have 
> the header's dataoffset change.  We have to be able to change the 
> dataoffset or we won't be able to read points if the caller passed in
>  any old value.

This looks OK, but perhaps could be better :-)

First, while creating reader user gets his input data modified.
Such things always smell, regardless of good intentions behind, and
reminds me things I don't like in GDAL (i.e. importFromWkt constructor
in OGRGeometry).

Second, if LASReader constructor signature is this:

LASReader(std::istream& ifs, LASHeader const& header);

It does not mean that the header is not modified. LASReader is free to
make a copy of it and inside it can do with this copy whatever LASReader
wants. User must not have any knowledge if his header is copied or
the reader keeps reference the original.


I would do it this way:

1. take header pass as input-only read-only parameter and make a copy:

LASReader(std::istream& ifs, LASHeader const& header) : m_header(header)
{
}

2. User knows (from docs) that LASReader can update header any time and
at any point of sequence of read steps. Given that, user knows that
whenever he wants to rely on header used by LASReader during reading,
he should request for current header than use original passed to ctor

LASHeader header = ...
LASReader reader(ifs, header);
// I need to use a header value
if (reader.GetHeader().GetDataOffset() = ...)
{
}

Alternatively, whenever user needs he can check if his original
header expired:

if (header != reader.GetHeader())
{
   // here I know my original header expired
}


To summary, (non-)constness of the header input to LASReader ctor is
completely meaningless information as LASReader makes (in current
implementation) makes a copy. So, let's make the signature clean
and teach user that LASReader is free to update header any time it needs.

Does it make sense?

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org