Clean up tools/obabel.cpp

[e-kwsm]

• clang-format
• use continue appropriately to reduce nesting levels

[baoilleach]

Sounds good but I can't review this patch, as it mixes whitespace changes with code changes. If you keep these in separate commits, then we see what's changed at each step, but when you merge them we haven't any idea.

[e-kwsm]

Separated into commits.

[baoilleach]

Some of these commits involve syntax changes that change code that is working fine now into code that may only be supported by newer compilers. I'm not familiar enough with the details of every compiler version to know exactly when these features were introduced, and what version of gcc ships with what version of CentOS. Every time I see newer syntax in a PR I think to myself - well, how can I merge this if I don't know whether it will cause problems for people? Maybe @ghutchis has another take on this.

[ghutchis]

I'm not sure about the the {} notation, but I'm willing to discuss that on-list.

[dspoel]

Curly braces for constructor calls are fine since at least C++11 and have some advantages: https://stackoverflow.com/questions/15396124/calling-constructor-with-braces

[dspoel]

Most patches seem OK at a glance, although it is difficult to overview the if statement change.

As regards whitespace changes, in principle I support an automatic cleanup and uniformisation of the code, but the proposed whitespace format is not attractive in my eyes. It is too condensed. Below is an example from GROMACS that uses clang-format as well and which is much more readable IMHO.

bool SelectionData::initCoveredFraction(e_coverfrac_t type)
{
    coveredFractionType_ = type;
    if (type == CFRAC_NONE)
    {
        bDynamicCoveredFraction_ = false;
    }
    else if (!_gmx_selelem_can_estimate_cover(rootElement()))
    {
        coveredFractionType_     = CFRAC_NONE;
        bDynamicCoveredFraction_ = false;
    }
    else
    {
        bDynamicCoveredFraction_ = true;
    }
    coveredFraction_        = bDynamicCoveredFraction_ ? 0.0 : 1.0;
    averageCoveredFraction_ = coveredFraction_;
    return type == CFRAC_NONE || coveredFractionType_ != CFRAC_NONE;
}