[bind10-dev] config experiment 2

[JINMEI Tatuya / 神明達哉]

Here are some random comments about the experimental implementation.
Some of them may be too detailed at this stage.  If so, please feel
free to ignore them.

- Just a question: do we assume the use of XML or some tree structure
  as part the API spec?  At least methods like "addChild()" indicates
  the configuration is stored in some tree structure.  But if we
  possibly change the internal representation in the future, we might
  want to provide a more generalized interface.  Note: I'm not
  suggesting it, just wondering.

- we'll also need to be able to remove an element(s)?

- if the configuration is stored in a remote server, would we specify
  it as part of the IDENTIFIER (such as a URI) and use the same API
  like getValue()?  If so, it probably makes sense, but the remote
  version of the call could "block", right?  Should we consider how to
  handle blocking operations for these interfaces?

- getConfigPart() requires the caller to delete its return value,
  assuming getConfigPart() allocates memory using new.  I think such
  an implicit requirement should be avoided.  

- would we like to provide an accessor (get/set) to a specific config
  node (element)?

- I didn't understand this method:
        /*
         * Adds an empty element to the children of the current node
         */
        void addChild(std::string name);

  what's the "current" node?  do you mean the base (root) node?  Or is
  this an arbitrary node in the tree determined by some context?  BTW,
  in general, should we treat the base node separately?  That is,
  since we could specify the path to the base node and use the more
  generic version of methods, aren't the "base-only" methods
  redundant?   (even if so I'm not necessarily objecting to it; if,
  for example, operations on the base node is much more common, it
  would make sense to provide convenient shortcuts)

- use of exception:
 * If the element or node is not found, usually a ConfigError exception
 * is thrown.
 I myself don't necessarily object to this approach, but I thought we
 previously loosely agreed we don't use exceptions for "expected
 exceptions", that is, non-bug exceptional events.  for example, we
 wouldn't throw an exception if we receive a malformed DNS message.
 Likewise, we'd handle it as a normal event if a user mistype a config
 parameter.  Again, I don't stick to this policy (or I may
 misunderstand the previous consensus), and I'm open to other
 approaches.  But in any case, I think we should have some common
 guideline on how/when to use exceptions.

- (especially if we decide to use exceptions heavily) the current
  prototype catches exceptions by value:

        } catch (ConfigError ce) {
            cout << "Caught ConfigError: " << ce.what() << endl;
        }

  but I'd catch it by reference.

        } catch (ConfigError& ce) {
            cout << "Caught ConfigError: " << ce.what() << endl;
        }

  see Item 13 of "more effective C++" for the reason.

- please make sure that all member variables are correctly initialized
  in constructors.  (don't assume these are set to 0/NULL by default).
  see my previous message about this.

- ~Config(): (you probably know this and this is an exceptional case,
  but anyway) you don't need the check to see if parser is non NULL
  before delete it.  delete is *smarter* than free:-)

- style issue: the prototype code adopts the "lowerThenUpper" style
  for member functions.  In particular, it uses getXXX() and setYYY()
  for accessors.  (In my understanding) we've not fixed a specific
  style on the case, but according to the current coding guideline
  http://bind10.isc.org/wiki/CodingGuidelines
  accessors would look like get_xxx()/set_yyy().  It would generally
  mean we use the "lower_underscore_lower" style.  I don't have a
  strong opinion on this (Evan said he liked the latter style), but we
  should have a unified, consistent style for production version of
  development.

- style issue: would we allow C-style comments?

---
JINMEI, Tatuya