BIND 10 #2384: extend RRTTL "from text" constructor so it can accpet 1H, 2D, etc.

[BIND 10 Development]

#2384: extend RRTTL "from text" constructor so it can accpet 1H, 2D, etc.
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121106
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  libdns++                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  loadzone-ng                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 I've not completed my review, but I'm giving some initial feedback.

 > It turned out the current master file loader is able to load TTLs with
 the
 > units now. However, I think it is not big enough feature to be noted in
 the
 > changelog directly, I think we'll include one big changelog once we
 complete
 > the loading. Would that be OK?

 I think it's worth a separate entry as it's an extension to a public
 API (RRTTL class itself is public), but I'd leave it to you.

 - I made a few minor editorial fixes myself.
 - I'd like to specify the specific text that causes the error in
   exception messages, like:
 {{{#!cpp
         isc_throw(InvalidRRTTL, "invalid TTL: " << ttlstr);
 }}}

 - BIND 9 rejects the "unit only form", like "1H" meaning "H".  If we
   have a specific reason for allowing that, it'd be better to be
   consistent.
 - If I read it correct this version allows an empty string.  I guess
   it should be rejected explicitly.
 - maybe doesn't matter much, but the I wonder whether we want to avoid
   the for loop within while.  For example, if we use a helper function
   that returns `multiply` in a switch, we can avoid examining all
   possibility (and I assume such a function is small enough and
   inlined).
 - I also wonder whether the construction of the string could be
   slightly more efficient:
 {{{#!cpp
                                   boost::lexical_cast<int64_t>(string(pos,
 unit));
 }}}
   if we use ttlstr::substr(), assuming it uses reference counting copy
   (I guess the above version always allocates memory).  I'd also note
   that the construction always happens even if there's no unit and we
   could actually use the original ttlstr.

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