BIND 10 #2955: DHCP-DDNS : Create DHCP-DDNS server class

[BIND 10 Development]

#2955: DHCP-DDNS : Create DHCP-DDNS server class
-------------------------------------+-------------------------------------
            Reporter:  stephen       |                        Owner:  tmark
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp          |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130606
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => tmark


Comment:

 Reviewed commit a9b9f641970b7f2db1b1183fab3deaec454b5b75

 '''src/bin/d2/d2_messages.mes'''[[BR]]
 > Since the module name is b10-dchp-ddns, having that again in the log is
 not much benefit. I'd like to stick with the D2CTL_ and D2PRC_ for now.
 The problem here is that the messages are read by the user.  "D2xxx" is OK
 for source code etc. as anyone who gets that far is a developer and will
 soon get into the way of thinking.  But the general user will be puzzled
 as to what "D2" is.

 In part this has been overcome in that the text of the first three
 messages starts "DHCP-DDNS xxx", so identifying the module.  But here, the
 long string is going into the message text rather than the identification
 so there is not a lot of gain.  (And to be consistent, DHCP-DDNS has to go
 into the text of all messages.)

 Arguably, adding "DHCP-DDNS" to the start of all messages is not needed
 given that all have an explanation in which the process name can be put.
 This suggests that the explanation associated with all messages should
 include the name of the relevant process, "b10-dhcp-ddns".  (BTW, one
 change I missed in the last review was that the explanation of some of the
 messages talk about "D2": that should be replaced by "b10-dhcp-ddns".)
 Against this is that the point that the first contact the user will have
 of the message is the appearance in the log (or message file) of message
 identification plus text. Ideally this should be descriptive enough for
 them to guess as to what the message is about, which means that
 restricting the process name to the message explanation is not enough. So
 we are back to including "DHCP-DDNS" somewhere in the identification or
 message text.

 Finally, it seems that all these contortions are taking place to avoid a
 message prefix starting "DHCPDDNS_XXX".  This is not overly long: the
 "stats-httpd" process uses a prefix of STATSHTTPD, so the precedent has
 been set for a message identification of such a length.


 '''src/bin/d2/d_process.h'''[[BR]]
 shouldShutDown(): Should the "D" be capitalised in "!ShutDown"?  Elsewhere
 the word is used without a capital "D" (except in the member variable name
 which is shut_down_flag).

 shouldShutDown(): parentheses needed in the "return" statement.

 getName(): should be declared "const" on the grounds that a const
 reference is returned so the class cannot be changed.

 shut_down_flag: see above for a comment about the name.  Also, as a member
 variable, the name should terminate with an underscore.

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