Write the coding-style guide for Modmesh #362

ThreeMonth03 · 2024-06-10T06:09:35Z

I add the draft about the coding-style.
There are some detail need to discuss.

Finish the draft of coding style

ThreeMonth03

I draft a coding style, and I have some question.

ThreeMonth03 · 2024-06-10T06:18:44Z

contrib/CODINGSTYLE.md

+3. The QT-related function name uses Camel case,  
+whereas non-QT-related function name uses snake case.
+4. The name of the class member starts with ```m_```.
+5. The name of [macros](https://en.cppreference.com/w/cpp/preprocessor/replace) starts with ```DECL_MM_```.


According to @tigercosmos and my suggestion,
here is the brief and rough conclusion.

However, it seems that the naming of the macros is inconsistent.

Is there any naming rule about macros?

interesting, @yungyuc could you decide a prefix name and let's change all the other?

Let's use MM_DECL*.

ThreeMonth03 · 2024-06-10T06:22:12Z

contrib/CODINGSTYLE.md

+### Formatting
+1. It is necessary to obey the [clang-format](https://clang.llvm.org/docs/index.html), or you must fail the github action.  
+2. There exists a space between the definition of the classes/functions.
+3. A long line should be devided to several shorter lines.
+
+### Class
+1. The class should follow [the rule of five](https://en.cppreference.com/w/cpp/language/rule_of_three).
+
+### Functions
+1. The recursive function should be replaced with the iterative function, unless it is inevitable to be used.


Should I add more concrete example or basic suggestion(like const member function/ copy by const reference) for the items?

I don't know we have such a rule.

This does not look like a coding style. It is a performance (HPC) concern. It should not be included in this file.

ThreeMonth03 · 2024-06-10T06:33:06Z

I add the draft about the coding-style. There are some detail need to discuss.

@yungyuc If you are availabe, may you start a review?

tigercosmos · 2024-06-10T07:50:53Z

contrib/CODINGSTYLE.md

+* [Functions](#functions)
+
+### Naming
+1. The class names use Camel case, such as ```class R3DWidget;```. 


R3DWidget is not a good example. QT related and non-QT can be different.
Also, let's clarify if we want HTTPRequest or HttpReuqest.

Correct. Please distinguish Qt and non-Qt classes. If not mentioned, the style refers to non-Qt classes, and you should take non-Qt classes for example.

R3DWidget is not a good example. QT related and non-QT can be different. Also, let's clarify if we want HTTPRequest or HttpReuqest.

Sorry, I don't get it.
How does the cpp program call the HTTPRequest or HttpReuqest function?

R3DWidget is not a good example. QT related and non-QT can be different. Also, let's clarify if we want HTTPRequest or HttpReuqest.

By the way, do you mean that if the class use HTTPRequest or HttpReuqest,
the class belongs to a QT-related classes?

R* classes are Qt classes in modmesh. I name them with the leading R because it's the next letter of Q. I do not want to give them the leading letter Q which is already used by Qt itself.

R3DWidget is a modmesh-made Qt derived class. The CamelCase rule is ambiguous for it because "3D" is an acronym for "three-dimensional" but "3" cannot be capitalized. Both R3DWidget and R3dWidget are CamelCased.

@tigercosmos used HTTPRequest and HttpRequest to explain that you need to consider how to deal with acronyms. The former keeps each of the letter in the acronym "HTTP" in upper-case. The latter treats the whole acronym as a word, and makes it "Http".

Treating an acronym as a word will make naming more consistent. So for acronyms, let's go with HttpRequest.

tigercosmos · 2024-06-10T07:51:31Z

contrib/CODINGSTYLE.md

+1. The class names use Camel case, such as ```class R3DWidget;```. 
+2. The name of the variable / using-declarations uses snake case,  
+such as ```R3DWidget * viewer;``` and ```using size_type = std::size_t;```. 
+3. The QT-related function name uses Camel case,  


what's the difference with point 1.

I notice that whether the class is related QT, the naming of the class is Camel case.

Class names are mostly CamelCase, although exceptions are not rare when it comes to a helper class.

Class names are mostly CamelCase, although exceptions are not rare when it comes to a helper class.

What is the style of the helper class?
Should I mention the helper class in this manual?

It's complicated

It's complicated

Ok, we may skip the helper class temporaily.

tigercosmos · 2024-06-10T07:52:16Z

contrib/CODINGSTYLE.md

+such as ```R3DWidget * viewer;``` and ```using size_type = std::size_t;```. 
+3. The QT-related function name uses Camel case,  
+whereas non-QT-related function name uses snake case.
+4. The name of the class member starts with ```m_```.


Suggested change

4. The name of the class member starts with ```m_```.

4. The name of the class member starts with `m_`.

What is the difference between ` and ```?

` is for inline code, ``` is for code block

@ThreeMonth03 check https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax

tigercosmos · 2024-06-10T07:52:44Z

contrib/CODINGSTYLE.md

+whereas non-QT-related function name uses snake case.
+4. The name of the class member starts with ```m_```.
+5. The name of [macros](https://en.cppreference.com/w/cpp/preprocessor/replace) starts with ```DECL_MM_```.
+6. The [using-declarations](https://en.cppreference.com/w/cpp/language/using_declaration) ends with ```_type```.


Suggested change

6. The [using-declarations](https://en.cppreference.com/w/cpp/language/using_declaration) ends with ```_type```.

6. The [using-declarations](https://en.cppreference.com/w/cpp/language/using_declaration) ends with `_type`.

tigercosmos · 2024-06-10T07:53:44Z

contrib/CODINGSTYLE.md

+### Formatting
+1. It is necessary to obey the [clang-format](https://clang.llvm.org/docs/index.html), or you must fail the github action.  
+2. There exists a space between the definition of the classes/functions.
+3. A long line should be devided to several shorter lines.


what's the recommended length? 120?

No, we have not been able to decide what is a proper length. @ThreeMonth03 , could you please make it explicit that we do not yet have a set limitation on line width?

note that for python code, the length limit is 80 for sure.

tigercosmos · 2024-06-10T07:56:38Z

@ThreeMonth03 you should mention the issue number when you raise the PR.

yungyuc · 2024-06-10T11:52:52Z

This PR references #350

yungyuc

Rename the file to /CODINGSTYLE.md
Remove Introduction
Distinguish non-Qt and Qt code
Review all indentation, blank lines, and markdown formatting
Remove recursion words

yungyuc · 2024-06-10T11:54:11Z

contrib/CODINGSTYLE.md

@@ -0,0 +1,43 @@
+# Coding Style
+## Introduction


I don't think we need this section whose only content is the table of contents.

yungyuc · 2024-06-10T11:55:21Z

contrib/CODINGSTYLE.md

Please rename to /CODINGSTYLE.md. The directory contrib/ is for code not for how to write code.

yungyuc · 2024-06-10T11:56:48Z

contrib/CODINGSTYLE.md

+* [Functions](#functions)
+
+### Naming
+1. The class names use Camel case, such as ```class R3DWidget;```. 


Correct. Please distinguish Qt and non-Qt classes. If not mentioned, the style refers to non-Qt classes, and you should take non-Qt classes for example.

yungyuc · 2024-06-10T11:57:50Z

contrib/CODINGSTYLE.md

+### Naming
+1. The class names use Camel case, such as ```class R3DWidget;```. 
+2. The name of the variable / using-declarations uses snake case,  
+such as ```R3DWidget * viewer;``` and ```using size_type = std::size_t;```. 


Please use perfect indentation of markdown. And "```" is rst, not markdown.

yungyuc · 2024-06-10T11:58:37Z

contrib/CODINGSTYLE.md

+3. The QT-related function name uses Camel case,  
+whereas non-QT-related function name uses snake case.
+4. The name of the class member starts with ```m_```.
+5. The name of [macros](https://en.cppreference.com/w/cpp/preprocessor/replace) starts with ```DECL_MM_```.


Let's use MM_DECL*.

yungyuc · 2024-06-10T12:00:42Z

contrib/CODINGSTYLE.md

+3. It is highly recommended to add the reference in the comment block.
+
+### Formatting
+1. It is necessary to obey the [clang-format](https://clang.llvm.org/docs/index.html), or you must fail the github action.  


clang-format is enforced by CI already. I don't think we need to write it down in the style guide.

yungyuc · 2024-06-10T12:01:55Z

contrib/CODINGSTYLE.md

+
+### Formatting
+1. It is necessary to obey the [clang-format](https://clang.llvm.org/docs/index.html), or you must fail the github action.  
+2. There exists a space between the definition of the classes/functions.


Do you mean a blank line by "space"?

yungyuc · 2024-06-10T12:03:26Z

contrib/CODINGSTYLE.md

+### Formatting
+1. It is necessary to obey the [clang-format](https://clang.llvm.org/docs/index.html), or you must fail the github action.  
+2. There exists a space between the definition of the classes/functions.
+3. A long line should be devided to several shorter lines.


No, we have not been able to decide what is a proper length. @ThreeMonth03 , could you please make it explicit that we do not yet have a set limitation on line width?

yungyuc · 2024-06-10T12:09:43Z

contrib/CODINGSTYLE.md

+3. A long line should be devided to several shorter lines.
+
+### Class
+1. The class should follow [the rule of five](https://en.cppreference.com/w/cpp/language/rule_of_three).


The rule of five is not exactly a coding style. Or do you mean to explicit write = default and = delete as much as possible?

yes, that's what I meant.

yungyuc · 2024-06-10T12:18:49Z

contrib/CODINGSTYLE.md

+### Formatting
+1. It is necessary to obey the [clang-format](https://clang.llvm.org/docs/index.html), or you must fail the github action.  
+2. There exists a space between the definition of the classes/functions.
+3. A long line should be devided to several shorter lines.
+
+### Class
+1. The class should follow [the rule of five](https://en.cppreference.com/w/cpp/language/rule_of_three).
+
+### Functions
+1. The recursive function should be replaced with the iterative function, unless it is inevitable to be used.


This does not look like a coding style. It is a performance (HPC) concern. It should not be included in this file.

Test

ThreeMonth03

Rename the file to /CODINGSTYLE.md
Remove Introduction
Distinguish non-Qt and Qt code
Review all indentation, blank lines, and markdown formatting
Remove recursion words

ThreeMonth03 · 2024-06-17T09:00:46Z

CODINGSTYLE.md

+# Coding Style
+


I remove the introduction, and modify the filename to CODINGSTYLE.md.

ThreeMonth03 · 2024-06-17T09:02:44Z

CODINGSTYLE.md

+1. Class names use CamelCase, for example: `class CallProfiler;`.
+   - Qt-related classes prepend an additional `R`, like `class RLine;`.
+     * A class is considered Qt-related if its parent class belongs to the Qt library.
+
+2. Variable names and using-declarations use snake_case:
+   - Example variable: `R3DWidget * viewer;`
+   - Example using-declaration: `using size_type = std::size_t;`
+
+3. Qt-related function names use CamelCase, while non-Qt-related function names use snake_case.
+   - Functions are Qt-related if they belong to a Qt-related class.
+   - Example Qt-related function: `QMdiSubWindow * RManager::addSubWindow(Args &&... args);`
+   - Example non-Qt-related function: `void initialize_buffer(pybind11::module & mod);`


I distinquish and define the Qt-related classes and functions in the list.
Additionally, I reformat the file.

ThreeMonth03 · 2024-06-17T09:03:17Z

CODINGSTYLE.md

+7. Acronyms within names should be treated as words instead of using ALL_CAPS_CONSTANTS.
+   - Example classes: `class HttpRequest;`
+   - Example function: `void update_geometry_impl(StaticMesh const & mh, Qt3DCore::QGeometry * geom);`


Here is the naming rule about acronyms.

ThreeMonth03 · 2024-06-17T09:07:26Z

CODINGSTYLE.md

+
+## Class
+
+1. It's advisable to explicitly define constructors and destructors for classes whenever possible.


According to the discussion, I modify the rule of five.

ThreeMonth03 · 2024-06-17T09:12:36Z

Rename the file to /CODINGSTYLE.md

Remove Introduction

Distinguish non-Qt and Qt code

Review all indentation, blank lines, and markdown formatting

Remove recursion words

@yungyuc I finish the modification of the document, but I'm not sure whether it could be code review.
I only modified the document and synchronized the latest main branch,
however, the github workflow failed.

yungyuc · 2024-06-17T10:22:01Z

Thanks, @ThreeMonth03 . The CI failure might be #365 , for which I just merged the fix @tigercosmos made in #367 . Could you please rebase to rerun the CI once the merged code has CI passed?

Sync and fix the bug in CICD.

ThreeMonth03 · 2024-06-23T11:46:39Z

Thanks, @ThreeMonth03 . The CI failure might be #365 , for which I just merged the fix @tigercosmos made in #367 . Could you please rebase to rerun the CI once the merged code has CI passed?

@yungyuc
Thank you for your suggestion.
The latest commit is merged, and the pr could be reviewed.

yungyuc

For Qt just say Qt. No need to be "-related".
When it's type alias, say type alias. Do not mistake with using-declaration.
Wording to distinguish C++ and Python.
More accurate title is "Constructors", not "Class".

yungyuc · 2024-06-23T12:31:22Z

CODINGSTYLE.md

+## Naming
+
+1. Class names use CamelCase, for example: `class CallProfiler;`.
+   - Qt-related classes prepend an additional `R`, like `class RLine;`.


Just say "Qt classes". It is redundant to write "-related".

yungyuc · 2024-06-23T12:32:15Z

CODINGSTYLE.md

+   - Qt-related classes prepend an additional `R`, like `class RLine;`.
+     * A class is considered Qt-related if its parent class belongs to the Qt library.
+
+2. Variable names and using-declarations use snake_case:


It is incorrect to say using-declarations. The use case you mentioned is type aliasing: https://en.cppreference.com/w/cpp/language/type_alias . Please correct.

yungyuc · 2024-06-23T12:32:50Z

CODINGSTYLE.md

+   - Example Qt-related function: `QMdiSubWindow * RManager::addSubWindow(Args &&... args);`
+   - Example non-Qt-related function: `void initialize_buffer(pybind11::module & mod);`
+
+4. Class members begin with `m_`.


yungyuc · 2024-06-23T12:36:16Z

CODINGSTYLE.md

+
+5. [Macros](https://en.cppreference.com/w/cpp/preprocessor/replace) start with `MM_DECL_`.
+
+6. [Using-declarations](https://en.cppreference.com/w/cpp/language/using_declaration) end with `_type`.


I think you mistake with https://en.cppreference.com/w/cpp/language/type_alias .

yungyuc · 2024-06-23T12:36:51Z

CODINGSTYLE.md

+   }; /* end of modmesh */
+   ```
+
+3. It is highly recommended to include references in comment blocks.


Just say it is recommended.

yungyuc · 2024-06-23T12:37:28Z

CODINGSTYLE.md

+   - For C++, no specific line length limit is set.
+   - For Python, adhere to an 80-character limit per line.
+
+## Class


It's more accurate to use "Constructors" as the subject.

tigercosmos · 2024-06-25T04:26:25Z

@ThreeMonth03 why closed?

yungyuc · 2024-06-25T07:23:32Z

@ThreeMonth03 why closed?

@ThreeMonth03 please make complete and explicit arguments before closing the PR.

ThreeMonth03 · 2024-06-25T08:38:25Z

@ThreeMonth03 why closed?

@yungyuc @tigercosmos
According to the suggestion of the @yungyuc ,
he suggest that I could solve the issue343 at first,
and close this PR.
Here is the new PR for issue 343.

yungyuc · 2024-06-25T08:45:41Z

@ThreeMonth03 You confuse people by referring to the private discussions between you and me. Please make an argument instead of delegating the reasoning to information that is not made public.

ThreeMonth03 · 2024-06-25T10:47:52Z

You confuse people by referring to the private discussions between you and me. Please make an argument instead of delegating the reasoning to information that is not made public.

@yungyuc I feel terribly sorry for my imprecise argument.

@tigercosmos
I think I'm unable to solve this issue now,
and I'm dealing with the issue343.
Thus, I decide to close the PR.
I'm sorry for closing the PR without any comment.

tigercosmos · 2024-06-25T11:02:57Z

@ThreeMonth03 it's okay to keep the PR open until you can continue, but it's also okay to reopen it when you are available.

ThreeMonth03 and others added 2 commits June 10, 2024 13:39

Finish the draft of coding style

4ebd550

Merge pull request #4 from ThreeMonth03/test

0595df0

Finish the draft of coding style

ThreeMonth03 commented Jun 10, 2024

View reviewed changes

ThreeMonth03 mentioned this pull request Jun 10, 2024

Document the coding style #350

Closed

tigercosmos reviewed Jun 10, 2024

View reviewed changes

yungyuc assigned ThreeMonth03 Jun 10, 2024

yungyuc added the documentation Improvements or additions to documentation label Jun 10, 2024

yungyuc requested changes Jun 10, 2024

View reviewed changes

ThreeMonth03 and others added 3 commits June 17, 2024 16:24

format and enhance the content of the codingstyle.md

cfc0736

Merge remote-tracking branch 'origin' into test

4fc0850

Merge pull request #5 from ThreeMonth03/test

6326549

Test

ThreeMonth03 commented Jun 17, 2024

View reviewed changes

ThreeMonth03 added 2 commits June 23, 2024 18:47

Merge branch 'solvcon:master' into test

5cca9ac

Merge pull request #6 from ThreeMonth03/test

9177d7d

Sync and fix the bug in CICD.

yungyuc requested changes Jun 23, 2024

View reviewed changes

ThreeMonth03 closed this Jun 25, 2024

ThreeMonth03 reopened this Jun 25, 2024

ThreeMonth03 closed this Jun 25, 2024

ThreeMonth03 deleted the debug branch July 6, 2024 18:59

yungyuc mentioned this pull request Jul 24, 2024

Add STYLE.rst for coding style guide #394

Merged

	4. The name of the class member starts with ```m_```.
	4. The name of the class member starts with `m_`.

	6. The [using-declarations](https://en.cppreference.com/w/cpp/language/using_declaration) ends with ```_type```.
	6. The [using-declarations](https://en.cppreference.com/w/cpp/language/using_declaration) ends with `_type`.


		## Class

		1. It's advisable to explicitly define constructors and destructors for classes whenever possible.


		5. [Macros](https://en.cppreference.com/w/cpp/preprocessor/replace) start with `MM_DECL_`.

		6. [Using-declarations](https://en.cppreference.com/w/cpp/language/using_declaration) end with `_type`.

Write the coding-style guide for Modmesh #362

Write the coding-style guide for Modmesh #362

Conversation

ThreeMonth03 commented Jun 10, 2024

ThreeMonth03 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThreeMonth03 commented Jun 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThreeMonth03 Jun 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThreeMonth03 Jun 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tigercosmos commented Jun 10, 2024

yungyuc commented Jun 10, 2024

yungyuc left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThreeMonth03 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThreeMonth03 commented Jun 17, 2024 • edited Loading

yungyuc commented Jun 17, 2024

ThreeMonth03 commented Jun 23, 2024

yungyuc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tigercosmos commented Jun 25, 2024

yungyuc commented Jun 25, 2024

ThreeMonth03 commented Jun 25, 2024 • edited Loading

yungyuc commented Jun 25, 2024

ThreeMonth03 commented Jun 25, 2024

tigercosmos commented Jun 25, 2024

ThreeMonth03 Jun 16, 2024 •

edited

Loading

ThreeMonth03 Jun 16, 2024 •

edited

Loading

yungyuc left a comment •

edited

Loading

ThreeMonth03 commented Jun 17, 2024 •

edited

Loading

ThreeMonth03 commented Jun 25, 2024 •

edited

Loading