2011/10/25 Peder Axensten <[email protected]>:
> I think we agree that Point.GetClassification() and Point.SetClassification( 
> ... )
> should not filter the Classification in any way?

Yes.

> (I don't really understand why there are 3 SetClassification() as 
> Classification
> has constructors to cover the other two cases, but never mind.)

The concept of classification is based on set of bits (flags).
We implemented it using std::bitset.
The set of constructors is defined to be a bit close to std::bitset,
with a bit of API discoverability in mind.

> I would think that the bug is that the right hand assignment expression of 
> the various
> Point.SetClassification(...) is Classification(...).GetClass().

Yes.

> The correct would be static_cast<bitset>(Classification(...)) or something 
> similar. Do you agree?

Nope.

I presented bug and fix here: https://gist.github.com/1313176

> As of retrieving all bits from a Classification object I had not noticed the 
> operator bitset()
> member function. I guess static_cast<uint8_t>(static_cast<bitset>( 
> p.GetClassification() ) )
> will work for me, with no execution overhead.

There is no conversion/cast from bitset to int.
You have to use bitset::to_ulong() function, then cast returned value
to uint8_t.
Here is long story step by step:

bitset<8> bits = p.GetClassification();
unsigned long ulong = bits.to_ulong();
uint8_t u8 = static_cast<uint8_t>(ulong);

You can do in one line:

static_cast<uint8_t>(bitset<8>(p.GetClassification()).to_ulong())

Note, here functional notation (which I prefer with user-defined types):
bitset<8>(p.GetClassification())
is equivalent to
static_cast<bitset<8>>(p.GetClassification())

> Although I'd be tempted to add an operator uint8_t() member function, for 
> symmetry, as there is such a constructor.

No more conversion operators please, especially to uint8_t which would
increase ambiguity.
In fact, the bitset conversion operator should never make it to the
interface of Classification, IMHO.
(There are very good reasons why std::bitset does not provide
conversion operators!)

IMO, the Classification should follow bitset::to_ulong, namely it
could have Classification::to_uint8().

I'm leaving Howard with decision what approach to use here.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
Member of ACCU, http://accu.org
_______________________________________________
Liblas-devel mailing list
[email protected]
http://lists.osgeo.org/mailman/listinfo/liblas-devel

Reply via email to