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

Coding style discussion #3

Open
szledan opened this issue Sep 19, 2015 · 9 comments
Open

Coding style discussion #3

szledan opened this issue Sep 19, 2015 · 9 comments

Comments

@szledan
Copy link
Member

szledan commented Sep 19, 2015

If you have any question or feedback about the project's coding style, please feel free to comment here.

@kkristof
Copy link
Member

kkristof commented Apr 3, 2016

Hi, I've found that the codestyle is a bit vague in terms of the comments.
As we would like to use the Doxygen we have to decide which comment style is preferred.
Here you can find which comment styles can be processed by the Doxygen: https://www.stack.nl/~dimitri/doxygen/manual/docblocks.html

I think for multi lines comments there are two option:

  • Java style:
\**
 * @brief One liner description
 *
 * Description...
 * @param example parameter
 *\

\** One line description *\
  • Qt style:
\*!
 * \brief One liner description
 *
 * Description...
 * \param example parameter
 *\

\*! One line description *\

For me the Qt style seems nicer.
What do you think about it?

@szledan
Copy link
Member Author

szledan commented Apr 4, 2016

I'd prefer the Java style.
Are there any reasons why we should choose the Qt style?

@kkristof
Copy link
Member

kkristof commented Apr 4, 2016

It's just a personal preference, for me the Qt's tag style seems nicer. But as the qtcreator (also personal preference) supports both so I'd fine with the java as well (probably it is widely supported in other IDE-s).
So if there are no objection we should use the java style and update the coding style.

@kkristof
Copy link
Member

I've added the critical label to this because we should improve the coding style as it lacks of necessary guidelines at the moment.
First of all we should discuss how to add multiline function declaration, and what line length we should use.

@szledan
Copy link
Member Author

szledan commented Apr 14, 2016

I think the best way if we need to break a (too long) line then use one indentation (4 whitespace) style.

void Gepard::drawImage(Image /*image*/, float sx, float sy, float sw, float sh,
    float dx, float dy, float dw, float dh)
{
    const float veryVeryVeryLongName = (sx * sh - sy * sw) * (sx * sh - sy * sw)
        + (dx * dh - dy * dw) * (dx * dh - dy * dw);
    const float anotherVeryVeryLongName = sqrt((sx * sw - sy * sh) * (sx * sw - sy * sh)
        + (dx * dw - dy * dh) * (dx * dw - dy * dh));
    const float notSoLongName = sx + sy + sw + sh + dx + dy + dw + dh;

    if (veryVeryVeryLongName + anotherVeryVeryLongName < notSoLongName
        && (veryVeryVeryLongName * anotherVeryVeryLongName > notSoLongName
            || veryVeryVeryLongName * anotherVeryVeryLongName * 2 > notSoLongName)
        || !notSoLongName)
    return;
}

A note: use the break in the reasonable position.

@szledan
Copy link
Member Author

szledan commented Apr 24, 2016

It is a 2. draft:

// About the line breaking.
// Philosophy:
//    1. Long lines are not a problem.
//    2. If breaking a line is necessary, please use the following suggestions.

template<class T>
static inline const T& clamp(const T& value, const T& min, const T& max)
{
    return (value < min) ?
        min :
        ((value > max) ?
            max :
            value);
}

// Too long argument list.
// Arrange the arguments in reasonable groups (if it is possible).
void arc(Float x, Float y,
         Float radius,
         Float startAngle = 0.0f, Float endAngle = 2.0f * piFloat,
         bool counterclockwise = false);
{
}

// If Doxygen comments are needed.
void Gepard::drawImage(Image image, /**< Image */
                       float sx = 0.0f, /**< Source x */
                       float sy = 0.0f, /**< Source y */
                       float sw = 0.0f, /**< Source width */
                       float sh = 0.0f, /**< Source height */
                       float dx = 0.0f, /**< Destination x */
                       float dy = 0.0f, /**< Destination y */
                       float dw = 0.0f, /**< Destination width */
                       float dh = 0.0f) /**< Destination height */
{
    // Too long line.
    // Arrange pieces in reasonable groups.
    const float veryLongName = (sx * sh - sy * sw) * (sx * sh - sy * sw)
                               + (dx * dh - dy * dw) * (dx * dh - dy * dw);
    // Extremely long line.
    const float anotherVeryVeryLongName =
        sqrt((sx * sw - sy * sh) * (sx * sw - sy * sh)
             + (dx * dw - dy * dh) * (dx * dw - dy * dh));

    if ((sx && sy && sw && sh && dx && dy && dw && dh)
        && ((sx - dx) && (sy - dy) && (sw - dw) && (sh - dh)))
    return;

    if (veryVeryVeryLongName + anotherVeryVeryLongName < notSoLongName
        && (veryVeryVeryLongName * anotherVeryVeryLongName > notSoLongName
            || veryVeryVeryLongName * anotherVeryVeryLongName * 2 > notSoLongName)
        || !notSoLongName)
    return;
}

This is a work in progress version. But please review and comment it.

@dbatyai
Copy link
Member

dbatyai commented Apr 24, 2016

+1, I like the second one.

@szledan
Copy link
Member Author

szledan commented May 1, 2016

About the Doxygen style.

I started to use the Java_style and I have to agree with @kkristof in the Qt's style.
So I found the reason (what I was missing earlier) for choosing this style and it is the clarity.

@kkristof
Copy link
Member

kkristof commented May 2, 2016

@szledan yes, I agree. For me the Qt style is easier to read, but all of us should agree with that. So let's call a vote. We have +2 on the Qt style against the Java style. Please share your opinion. :)

@szledan szledan added this to the continous milestone Sep 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants