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