BIND 10 #537: Make asiolink::UDPServer smaller to copy

BIND 10 Development do-not-reply at isc.org
Thu Feb 3 18:13:52 UTC 2011


#537: Make asiolink::UDPServer smaller to copy
-------------------------------------+-------------------------------------
                 Reporter:  vorner   |                Owner:  jinmei
                     Type:  task     |               Status:  reviewing
                 Priority:  major    |            Milestone:  A-Team-
                Component:           |  Sprint-20110209
  Unclassified                       |           Resolution:
                 Keywords:           |            Sensitive:  0
Estimated Number of Hours:  6.0      |  Add Hours to Ticket:  0
                Billable?:  1        |          Total Hours:  0
                Internal?:  0        |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:7 vorner]:

 > > I'm not sure if we want to have this level of tuning at this point,
 > > but since the changes are basically straightforward and not so big,
 > > I don't oppose to that.
 >
 > There's no time to do the proper tuning with redesigning and
 refactoring.

 Right, but my point is that there are certainly worse time to do
 tuning that would introduce premature optimizations.  We do not
 necessarily have to wait for "the right time to do proper tuning" but
 we should not do it in worse time.

 For refactoring (for such purposes as better readability), I basically
 agree it should be done as soon as we see possibility of it.

 > >   The original style is the (vaguely documented) convention in BIND 9
 > >   (see the "Indentation" section of
 > >   http://bind10.isc.org/wiki/BIND9CodingGuidelines).  As a fan of
 > >   consistency I'd like to keep a consistent style (either way) if
 > >   possible.  But this is quite a minor point.
 >
 > I didn't find it there anyway. But you're right this looked odd, I
 didn't update the indentation. I prefer indenting by 4 spaces as in any
 other nested statement, is that OK (as I changed it)? Stuff like this
 looks strange to me:
 >
 > {{{
 >          really_long->function->call(one_param,
 >                                    second,
 >                                    third ? or :
 >                                    another);
 > }}}

 I personally prefer the "strange" style, because it better clarifies
 the relationship between a function and its parameters.  With the
 "strange" style, we'll have

 {{{
     function1(function2(function3(arg_for_function3_1,
 arg_for_function3_2),
                         arg_for_function2_1, arg_for_function2_2),
               arg_for_function1_1, arg_for_function1_2);
 }}}

 and we can easily understand arg_for_functionN_M are for functionN.

 With the 4-space style, it would look:

 {{{
     function1(function2(function3(arg_for_function3_1,
 arg_for_function3_2),
         arg_for_function2_1, arg_for_function2_2), arg_for_function1_1,
         arg_for_function1_2);
 }}}

 which is confusing to me.  This is of course an artificial and extreme
 example, but with C++ such deeply nested expression is not uncommon.

 udpdns.cc has a more practical example:

 {{{
         CORO_YIELD data_->socket_->async_send_to(
             buffer(data_->respbuf_->getData(),
 data_->respbuf_->getLength()),
             *data_->sender_, *this);
 }}}

 If this is indented in the 4-space style, I cannot be sure whether or
 not "*data_->sender_" and "*this" are part of buffer() parameters.  In
 the "strange" style, it's obvious because otherwise it should look
 like this:

 {{{
         CORO_YIELD data_->socket_->async_send_to(
             buffer(data_->respbuf_->getData(),
 data_->respbuf_->getLength(),
                    *data_->sender_, *this));
 }}}

 Another reason why I prefer the "strange" style is that it's
 consistent with BIND 9 (and apparently at least with ISC DHCP also).

 But I also admit the "strange" style has disadvantage with longer
 lines (this style tends to require more lines when function parameters
 begin with a higher column), which is also quite common in C++.

 Like any other style matters, this is largely a matter of preference
 in the end, however, and I'm willing to adopt any agreed style.  This
 is probably a (bikeshed) topic to be discussed at bind10-dev?

 In any case, I don't think this is a blocking issue of this ticket.
 Feel free to move forward and merge.

-- 
Ticket URL: <https://bind10.isc.org/ticket/537#comment:9>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list