BIND 10 #1130: RR type implementation: NAPTR

[BIND 10 Development]

#1130: RR type implementation: NAPTR
-------------------------------------+-------------------------------------
                   Reporter:  shane  |                 Owner:  ocean
                       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 jelte):

 * owner:  jelte => ocean


Comment:

 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.

 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.

 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')

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