[bind10-dev] style thingy wrt unit test code coverage

[Evan Hunt]

> if (condition) { do_something; }
> 
> on one line will always be counted, because the condition is executed,
> but we might miss the fact that do_something might not be.
> 
> I don't know about the rest but i tend to put simple things like this on
> the same line, and perhaps we should make it a style issue to not do
> that, as the coverage result gets skewed.

I think with if statements, that's already covered in the BIND9 style
guide--and yeah, we shouldn't do it.

> The more interesting ones is the same issue in header files; 'simple'
> functions we can do there, but as soon as branching gets involved can we
> still call it simple?

IMHO it's okay to have a one-line declaration that does a simple return
or assignment:

        const int foo() const { return foo_; }
        void setFoo(int f) { foo_ = f; }

...but anything more complicated than that should be multi-line.

Speaking of bracket style, there doesn't seem to be any agreement on when
to open a bracket at the end of a line and when to move it down to a new
line.  I find this rather ugly:

    DNSKEYImpl(uint16_t flags, uint8_t protocol, uint8_t algorithm,
               const vector<char>& keydata) :
        flags_(flags), protocol_(protocol), algorithm_(algorithm),
        keydata_(keydata)
    {
        ...
    }

...but better on the whole than leaving the bracket on the line above,
causing the "keydata_(keydata)" line to be indented at the same level
as code immediately below it:

    DNSKEYImpl(uint16_t flags, uint8_t protocol, uint8_t algorithm,
               const vector<char>& keydata) :
        flags_(flags), protocol_(protocol), algorithm_(algorithm),
        keydata_(keydata) {
        ...
    }

And, speaking of indentation style and how it interacts with bracket style,
long if statements are problematic with our standard four-space indent:

    if (impl_->flags_ != other_dnskey.impl_->flags_ &&
        impl_->protocol_ != other_dnskey.impl_->protocol_ &&
        impl_->algorithm_ != other_dnskey.impl_->algorithm_) {
        do_some_stuff();
    }

Two proposed solutions:

    if (impl_->flags_ != other_dnskey.impl_->flags_ &&
           impl_->protocol_ != other_dnskey.impl_->protocol_ &&
           impl_->algorithm_ != other_dnskey.impl_->algorithm_) {
        do_some_stuff();
    }

    if (impl_->flags_ != other_dnskey.impl_->flags_ &&
        impl_->protocol_ != other_dnskey.impl_->protocol_ &&
        impl_->algorithm_ != other_dnskey.impl_->algorithm_)
    {
        do_some_stuff();
    }

Shall we agree on one of these?  (As a matter of taste, I think the first
one is slightly more attractive, but they both work okay.)

                                        eh