Add STYLE.rst for coding style guide #394
Conversation
The coding style guide includes file format, naming, import and include, and other conventions in modmesh code base.
There was a problem hiding this comment.
I added some inline review comments to help review. @j8xixo12 @tigercosmos @ThreeMonth03 @q40603
| Coding Style Guide | ||
| ==== | ||
|
|
||
| A code style guideline is to help developers align how they write and change |
| the former matters more than the latter, because the former costs more than the | ||
| latter. | ||
|
|
||
| modmesh uses `clang-format <https://clang.llvm.org/docs/ClangFormat.html>`__ to |
| file and follow it. Familiar with the code in the module(s) if time permits. | ||
| 3. Use the style guide. | ||
|
|
||
| Indentation and file format |
There was a problem hiding this comment.
File format and indentation.
|
|
||
| Use a blank line between the definitions of classes and functions. | ||
|
|
||
| Naming |
There was a problem hiding this comment.
Most things about naming.
| - ``clfcs(icl, ifl)``: get the ``ifl``-th face in ``icl``-th cell. | ||
| - ``fcnds(ifc, inf)``: get the ``inf``-th fact in ``ifc``-th face. | ||
|
|
||
| Python import |
|
|
||
| } /* end namespace modmesh */ | ||
|
|
||
| C Pre-Processor Macro |
| C++ Standard | ||
| ==== | ||
|
|
||
| Use C++-17 and beyond. |
|
|
||
| Use C++-17 and beyond. | ||
|
|
||
| Follow the `rule of five |
| ~MyClass() = default; | ||
| }; /* end class MyClass */ | ||
|
|
||
| C++ Encapsulation |
| It is OK for accessors of the same-name and getter-and-setter styles to be | ||
| available for the same member datum, but we should only do it when necessary. | ||
|
|
||
| C++ Ending Mark |
| - ``jfc`` means the second-level (iterating) index of face. | ||
| - Some special iterators used in code, such as: | ||
|
|
||
| - ``clfcs(icl, ifl)``: get the ``ifl``-th face in ``icl``-th cell. |
There was a problem hiding this comment.
Personally, I think these shorthands are hard to read.
There was a problem hiding this comment.
They are hard to read, but it is better than making the code so lengthy like:
psolution_derivative = alg->solution_derivative + jcl*NEQ*NDIM;
for (ieq=0; ieq<NEQ; ieq++)
{
solution_of_sub_face[ieq] = qdt * pj_solution_temporal[ieq];
solution_of_sub_face[ieq] += (p_sub_face_center[0][0]-pj_cell_center[0]) * psolution_derivative[0];
solution_of_sub_face[ieq] += (p_sub_face_center[0][1]-pj_cell_center[1]) * psolution_derivative[1];
}I tried it long-time ago and it made it so hard to debug. The shorthands look weird initially but they became much nicer when the developer gets used to the numerical method, which requires some weeks of derivation of the equations.
There was a problem hiding this comment.
Different modules may use different shorthands. Those who are familiar with the numerical methods and algorithms should discuss what shorthands to use and document in the style guide.
It is about scoping. No one knows everything. Productivity comes with keeping a proper scope for development and maintenance.
|
|
||
| } /* end namespace modmesh */ | ||
|
|
||
| Copyright Notice |
There was a problem hiding this comment.
I think copyright could be just a single line refering to the copyright file. It's annoying that a file has tens of line of code just for the copyright.
There was a problem hiding this comment.
I need the copyright texts to be in all files.
There was a problem hiding this comment.
I wonder what is the difference? According to what I read from the clean code book, we shouldn't copy the full copyright text everywhere.
There was a problem hiding this comment.
I need the texts in all files but would not like to provide the reasons. I cannot change the requirement now but I can explain more when I can.
The use cases of a scientific code are not always popular. Although the code is open source, many of the use cases may not be open.
There was a problem hiding this comment.
I have difficulty understanding why the amount of comments or notice left in a file may have anything to do with productivity, and do not understand why "clean code" is a support for having short copyright notice. But I am interested in your argument.
There was a problem hiding this comment.
I disagree that copyright notice distracts a developer, especially after we have written down the style guide. It is fairly easy to get used to skipping 25-30 lines of code in the beginning of the files after reading around 5 files of that kind.
A developer only gets distracted if they do not plan to spend time reading some files in the code base.
I wonder if the argument of "long headers distract" is from the "clean code" book?
There was a problem hiding this comment.
I realized my mistake. "Clean Code" advises against using multi-line comments at the beginning of a file for some cases but doesn't mention anything about copyright comments. The book "CODE 2" does discuss copyright comments, providing an example of a one-line copyright notice, but it doesn't state that long copyright comments are undesirable.
There was a problem hiding this comment.
It makes sense for a book to use a short form of copyright notice because paper-printing costs. If one line suffices for demonstration it does not need to cover the full license content. Moreover, a full content needs to pick a specific license agreement and the decision clearly goes outside the scope of a book for software engineering.
While we will not discuss the reasons behind the requirement of add the full license content in the file header, we can still discuss the pros and cons. The discussion and analysis benefit more than taking a context-free conclusion from a book.
There was a problem hiding this comment.
We must be careful on taking conclusions from a reference. Even Newton's laws of motion has a constraint on applicability. It is usually better to take an argument rather than (just) a conclusion out of a reference. It is not uncommon for a living project to contain richer context than a book.
Scientific and high-performance computing carries much more context than the software engineering, which does not take engineering into account. This fascinates for it takes more analytical faculty.
There was a problem hiding this comment.
I see. Good to know :)
|
The style guide describes what we are already doing in the code base. We can and should use more discussion but it does not change what has been established. I am merging it and keep discussion open. |
|
I created #395 for further discussions on the coding style. |
Thank you. The document is very clear and informative. It looks good to me. |
The coding style guide includes file format, naming, import and include, and other conventions in modmesh code base.
This PR is to follow up a previous PR #362 by @ThreeMonth03 for issue #350.