BIND 10 #1130: RR type implementation: NAPTR

[BIND 10 Development]

#1130: RR type implementation: NAPTR
-------------------------------------+-------------------------------------
                   Reporter:  shane  |                 Owner:  jelte
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:
                   Priority:  major  |  Sprint-20110816
                  Component:         |            Resolution:
  libdns++                           |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  4
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by ocean):

 * owner:  ocean => jelte


Comment:

 Replying to [comment:15 jelte]:
 > Looking good, but I have three final suggestions (sorry for that); it
 looks like skipLeftSpaces is only called from within the constructor, and
 it does not need any state, so we might as well change it from a private
 function to one in an anonymous namespace within the .cc file (and perhaps
 even an inline one). And I think the same goes for getNextCharacterString.
 >
 Done, I have moved them to anonymous namespace，maybe the
 {{{getNextCharacterString}}} should be moved to a utility library because
 it may be needed by other rrdata classes.
 BTW, Any suggestions are welcome and appreciated:)

 > Another thing is that skipLeftSpaces could probably throw the exception
 itself; then you could for instance do something like
 > {{{
 > NAPTR::skipLeftSpaces(const std::string& input_str,
 >                     std::string::const_iterator& input_iterator)
 > {
 >     if (!isspace(*input_iterator)) {
 >         isc_throw(InvalidRdataText,
 >                   "Invalid NAPTR text format, fields are not separated
 by space");
 >     }
 >     ...etc
 > }}}
 > and you do not need a temporary variable and a return value check,
 making the calling code much shorter.
 >
 Done
 > And if we are doing checks there anyway, just before skipLeftSpaces()
 returns, we might want to check if the iterator is not equals to
 input_str.end(), if so, it seems the input data is missing a field (with
 the current code, if you leave out one or more fields, the error will be
 'not separated by space')
 Done, doing the check before skipping.

-- 
Ticket URL: <http://bind10.isc.org/ticket/1130#comment:16>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development