[Liblas-devel] Some ideas for class organization

[Mateusz Loskot]

Philippe Vachon wrote:
> Hi everyone,
> 
> I threw together a quick trac page on how I might go about
> structuring libLAS, from the C++ perspective.
> 
> See: http://liblas.org/wiki/ClassStructure

Philippe,

I'd like to share my 5 groschen to the discussion :-)
Generally, I like the proposal but I have a few things I'd like to discuss.

First question, what format version does the implementation follow?

> I would be interested in any ideas/feedback/limitation you see. I 
> haven't really thought out the writing side of things, but I think
> that there will need to be an interface for the LASFile class to
> allow the various parameters of interest (offset, scaling factors,
> georeferencing information) be set, then written out when some sort
> of flush() method is called (and/or when the class' destructor is
> called, of course).

Is my understanding correct, that LASFile is the entity representing LAS
datasource?

1. LASPoint

I'd add constructor that will initialize members of the structure with
default values (Zero).

2. LASHeaderEntry

IMHO, there is lack of const-correctness, so I'd change its members
according to:

int GetRecordID() const { return nRecordID; }
int GetRecordLen() const { return nRecordLen; }
const char *GetUserID() const { return psUserID; }
const char *GetDescription() const { return pszDescription; }
const char *GetData() const { return pszData; }

Also, I think it's good to implement specification rules precisely, for
example:

int GetRecordID() const
{
   if (0 <= nRecordID && nRecordID < 65536)
      return nRecordID;

   throw std::out_of_range();
}

// May return NULL.
const char *GetDescription() const;

3. LASException

class LASException : public virtual std::exception
{
};


3.1. char *GetExceptionString();

What is the exception string and how does it differ from Long and Short
descriptions?

Can it be replaced with what is sent by std::exception::what() ?

3.2. const char *GetLongDescription(); and const char
*GetShortDescription();

I'm not sure if this is necessary to have complex such exception class
providing a lot of information. How would that be useful?

3.3. const char *GetFunctionName();

IMHO, this information is useless for library client.
It's useful for liblas developers but not for clients.
Otherwise, I'd dump such information into log file.

3.4. LASError GetErrorType() { return eType; }

How many error types do we have?
If there are not many types, we could consider representing different
types as separate exception class.


4. LASFile

Why C I/O instead of C++ I/O ?
If C I/O, why not CPL large file I/O?

4.1. Missing constness:

const LASHeaderEntry &GetHeaderRecord const
const LASPoint &GetPointByID const
const LASPoint &GetNextPoint const

4.2. GetNextPoint

How pass-the-end is recognized?
Personally, I'd use iterators, but this is not a problem. They can be
implemented as thin orthogonal layer that uses base LASFile interface.

4.3. WritePoint

Are we going to allow to change currently existing points?
If we are, then I'd redesign this interfaces as follows:

One WritePoint replaced with:
- long long AddPoint(const LASPoint &roPoint);
- void SetPoint(const LASPoint &roPoint, long long nPointID);


5. long long

This type is GCC-specific. I'd suggest to introduce our own type as
follows (schematic impl., more specific if-defs needed):

namespace liblas
{
#if HAVE_LONG_LONG
   typedef long long int_t;
#elif _MSC_VER
   typedef __int64 int_t;
#elif ...
   typedef ...;
#endif
}

6. I'd suggest to use C++ namespaces, for example:

namespace liblas
{

class PointData
{
  enum Classifications
  {
     eCreated = 0,  // never classified
     eUnclassified,
     eGround,
     eLowVegetation,
     ...
  };

  ... // class members
};

class HeaderEntry {};

class Exception : public virtual std::exception {};

class File {};

namespce details
{
  // all private stuff that does not belong to public
  // interface goes here.
} namespace liblas::details

} // namespace liblas

> The other thing I wasn't sure about was exception handling. I would
> like to go with structured exception handling, exposing a generic
> exception class perhaps to client applications so we can just throw
> that and let the application using the library catch it. Anyways,
> points to ponder, I guess?

Do you mean structured exceptions handling as SEH?
If you do, then I'm against this technique for one reason: this
technology is Microsoft specific.

If you mean C++ exceptions, then I think it's a good idea to use it.
Then, I'd propose to derive liblas exceptions tree from std::exception
class.

If we agree to use C++ exceptions, I'd recommend to follow the best
practice rules described by David Abrahams in "Error and Exception
Handling" (it is not boost-specific):
http://www.boost.org/more/error_handling.html
plus this one:
http://www.parashift.com/c++-faq-lite/exceptions.html


Cheers
-- 
Mateusz Loskot
http://mateusz.loskot.net