Open-Systems-Pharmacology / OSPSuite.FuncParser

Function parser for numeric and logic expressions used by the Open Systems Pharmacology Suite
Other
0 stars 3 forks source link

Follow a uniform code formatting convention? #54

Open IndrajeetPatil opened 2 years ago

IndrajeetPatil commented 2 years ago

There doesn't seem to be any unique formatting convention followed in the codebase, and restyling the document in Visual Studio leads to large changes stemming from formatting. I keep undoing such changes and stick only to coding-related changes when I am making a PR.

But that makes me wonder if it might be worth it to have a single PR that unifies formatting of all .cpp files using Visual Studio without any changes related to the code.

This will be akin to running {styler} on all .R files in an R package with legacy code that doesn't follow any styling guidelines, like the tidyverse style guide.

Yuri05 commented 2 years ago

If some of the source files(.cpp, .h, ...) do not use our recommended formatting - it would be a good idea to reformat once.

IndrajeetPatil commented 2 years ago

Actually, I didn't find any source files violating the recommended conventions, the formatting changes are mostly due to indentation.

For example, this file

#include "FuncParser/Math.h"

namespace FuncParserNative
{

Constant::Constant (const std::string & Name, double Value)
{
    _name = Name;
    _value = Value;
}

const std::string & Constant::GetName () const
{
    return _name;
}

const double Constant::GetValue () const
{
    return _value;
}

}//.. end "namespace FuncParserNative"

is styled to

namespace FuncParserNative
{

    Constant::Constant(const std::string& Name, double Value) :
        _name{ Name },
        _value{ Value }
    {
    }

    const std::string& Constant::GetName() const
    {
        return _name;
    }

    const double Constant::GetValue() const
    {
        return _value;
    }

}//.. end "namespace FuncParserNative"

I prefer the latter because it makes the nested scopes clearer based on indentation level, but not sure if that's in violation with the formatting guidelines.

msevestre commented 2 years ago

indent should be 3. This is in our guidelines no? It looks like you are using 4 or even 6? As for the namespace indent, it makes sense as well to me

msevestre commented 2 years ago

yes it is. Actual first item on the list https://github.com/Open-Systems-Pharmacology/Suite/blob/develop/CODING_STANDARDS.md#visual-studio-settings

IndrajeetPatil commented 2 years ago

As for the namespace indent, it makes sense as well to me

Yeah, this is what I was getting at. I wasn't sure if vertically aligning braces for different scopes was intentional or a relic of outdated coding standards.

msevestre commented 2 years ago

Some of this code was in fact generated automatically based on some UML diagrams back in the day...can't even remember what the name of the tool was lol