Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reformat bondcpp for better readability #52

Open
wants to merge 5 commits into
base: kinetic-devel
Choose a base branch
from

Conversation

kejxu
Copy link
Contributor

@kejxu kejxu commented Jul 12, 2019

reformatting bondcpp code according to https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines for better readability

  • indent with 4 spaces
  • place each function call parameter on separate line if they are broken into multiple lines
  • apply consistent coding style, this also provides clearer representation of a variable's type upon inspection
    • use type& name over type &name, the actual type of name is type&
    • use type* name over type *name, the actual type of name is type*

, and use early return for a clearer representation of logic:

void Bond::bondStatusCB(msg)
{
    if (msg->id == id_ && msg->instance_id != instance_id_)
    {
        // action
    }
}

has been changed to

void Bond::bondStatusCB(msg)
{
    if (msg->id != id_)
    {
        // filter out heartbeat from other bonds
        return;
    }
    if (msg->instance_id == instance_id_)
    {
        // filter out heartbeat from this bond
        return;
    }

    // action
}

this also reduces the number of layers in code, providing better readability

@kejxu
Copy link
Contributor Author

kejxu commented Jul 12, 2019

kinetic build was showing this error message:

error: ‘>>’ should be ‘> >’ within a nested template argument list

while melodic builds fine

afaik, this is unnecessary as long as >> is not actually overridden, that's probably why melodic build was okay (since it uses a newer version of g++ compiler). However, this does not seem like something that has enough priority to be considered at the point. Changing >> to > > in favor of kinetic build.

@kejxu kejxu mentioned this pull request Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant