GitHub PullRequestReviewCommentEvent at 2014-05-25 09:39:39
Repository: dolphin
Actor: neobrain
Payload: {"comment":{"url":"https://api.github.com/repos/dolphin-emu/dolphin/pulls/comments/13030024","id":13030024,"diff_hunk":"@@ -0,0 +1,218 @@\n+# Dolphin Code Styling\n+\n+## Table of Contents\n+---\n+\n+- [Introduction] (#introduction)\n+- [Styling and formatting] (#styling-and-formatting)\n+  - [General] (#general)\n+  - [Naming] (#naming)\n+  - [Conditionals] (#conditionals)\n+  - [Classes and Structs] (#classes-and-structs)\n+- [Code specific] (#code-specific)\n+  - [General] (#general-1)\n+  - [Headers] (#headers)\n+  - [Loops] (#loops)\n+  - [Functions] (#functions)\n+  - [Classes and Structs] (#classes-and-structs-1)\n+\n+\n+## Introduction\n+---\n+\n+This guide is for developers who wish to contribute to the Dolphin Emulator codebase. It will detail how to properly style and format code to fit this project. This guide also offers suggestions on specific functions and other varia that may be used in code.\n+\n+Following this guide and formatting your code as detailed will likely get your pull request merged much faster than if you don't (assuming the written code has no mistakes in itself).\n+\n+## Styling and formatting\n+---\n+\n+### General\n+- Try to limit lines of code to a maximum of 100 characters.\n+    - Note that this does not mean you should try and use all 100 characters every time you have the chance. Typically with well formatted code, you normally shouldn't hit a line count of anything over 80 or 90 characters.\n+- The indentation style we use is tabs for initial indentation and then, if vertical alignment is needed, spaces are to be used.\n+- The opening brace for namespaces, classes, functions, enums, structs, unions, conditionals, and loops go on the next line.\n+  - With array initializer lists and lambda expressions it is OK to keep the brace on the same line.\n+- Don't collapse single line conditional or loop bodies onto the same line as its header. Put it on the next line.\n+  - Yes:\n+\n+    ```c++\n+    if (condition)\n+        return 0;\n+\n+    while (var != 0)\n+        var--;\n+    ```\n+  - No:\n+\n+    ```c++\n+    if (condition) return 0;\n+\n+    while (var != 0) var--;\n+    ```\n+\n+### Naming\n+- All class, enum, function, and struct names should be in upper CamelCase. If the name contains an abbreviation, acronym, or initialism ? uppercase it.\n+  - `class SomeClassName`\n+  - `enum IPCCommandType`\n+- All compile time constants should be fully uppercased. With constants that have more than one word in them, use an underscore to separate them.\n+  - `const int PI = 3.14159;`\n+  - `const int MAX_PATH = 260;`\n+- All variables should be lowercase with underscores separating the individual words in the name.\n+  - `int this_variable_name;`\n+- Please do not use [Hungarian notation](http://en.wikipedia.org/wiki/Hungarian_notation) prefixes with variables.\n+- Variable prefixes\n+  - Global variables ? `g_`\n+  - Class variables ? `m_`\n+\n+### Conditionals\n+- Do not leave `else` or `else if` conditions dangling unless the `if` condition lacks braces.\n+  - What not to do:\n+\n+    ```c++\n+    if (condition)\n+    {\n+        // code\n+    }\n+    else\n+        // code line\n+    ```\n+  - What to do:\n+\n+    ```c++\n+    if (condition)\n+    {\n+        // code\n+    }\n+    else\n+    {\n+        // code\n+    }\n+    ```\n+  - Acceptable dangling:\n+\n+    ```c++\n+    if (condition)\n+        // code line\n+    else\n+        // code line\n+    ```\n+\n+### Classes and Structs\n+- If making a [POD](http://en.wikipedia.org/wiki/Plain_Old_Data_Structures) type, use a `struct` for this. Use a `class` otherwise.\n+- Class layout should be in the order, `public`, `protected`, and then `private`.\n+  - If one or more of these sections are not needed, then simply don't include them.\n+- For each of the above specified access levels, the contents of each should follow this given order: constructor, destructor, operator overloads, functions, then variables.\n+- When defining the variables, define `static` variables before the non-static ones.\n+\n+```c++\n+class ExampleClass : public SomeParent\n+{\n+public:\n+    ExampleClass(int x, int y);\n+  \n+    int GetX() const;\n+    int GetY() const;\n+\n+protected:\n+    virtual void SomeProtectedFunction() = 0;\n+    static float s_some_variable;\n+\n+private:\n+    int m_x;\n+    int m_y;\n+};\n+```\n+\n+## Code Specific\n+---\n+\n+### General\n+- Using C++11 features is OK and recommended.\n+- Use the [nullptr](http://en.cppreference.com/w/cpp/language/nullptr) type over the macro `NULL`.\n+- If a [range-based for loop](http://en.cppreference.com/w/cpp/language/range-for) can be used instead of container iterators, use it.\n+- Obviously, try not to use `goto` unless you have a *really* good reason for it.\n+- If a compiler warning is found, please try and fix it.\n+- Try to avoid using raw pointers (pointers allocated with `new`) as much as possible. There are cases where using a raw pointer is unavoidable, and in these situations it is OK to use them. An example of this is functions from a C library that require them. In cases where it is avoidable, the STL usually has a means to solve this (`vector`, `unique_ptr`, etc).\n+- Do not use the `auto` keyword everywhere. While it's nice that the type can be determined by the compiler, it cannot be resolved at 'readtime' by the developer as easily. Use auto only in cases where it is obvious what the type being assigned is (note: 'obvious' means not having to open other files or reading the header file). Some situations where it is appropriate to use `auto` is when iterating over a `std::map` container in a foreach loop, or to shorten the length of container iterator variable declarations.\n+- Do not use `using namespace [x];` in headers. Try not to use it at all if you can.\n+\n+### Headers\n+- If a header is not necessary in a certain source file, remove it.\n+- If you find duplicate includes of a certain header, remove it.\n+- When declaring includes in a source file, make sure they follow the given pattern:\n+  - Standard library headers\n+  - System-specific headers (these should also likely be in an `#ifdef` block unless the source file itself is system-specific).\n+  - Dolphin source file headers\n+- Headers should be included in a way that is relative to the `[Dolphin Root]/Source/Core` directory.\n+- The headers should be included in alphabetical order for each section respectively.\n+\n+### Loops\n+- If an infinite loop is required, do not use `for (;;)`, use `while (true)`.\n+- Empty-bodied loops should use braces after their header, not a semicolon.\n+  - Yes: `while (condition) {}`\n+  - No: `while (condition);`\n+\n+### Functions\n+- If a function parameter is not intended to have their assigned value or data changed, please mark that parameter as `const`. Local variables within functions that do not change value should also be marked accordingly as well.\n+- Always try to define `std::string` parameters as const references, unless the specific aim of the function is to modify said `std::string`.\n+- Functions that specifically modify their parameters should have the respective parameter(s) marked as a pointer so that the variables being modified are sytaxically obvious.\n+  - What not to do:\n+\n+    ```c++\n+    template<class T>\n+    inline void Clamp(T& val, const T& min, const T& max)\n+    {\n+        if (val < min)\n+            val = min;\n+        else if (val > max)\n+            val = max;\n+    }\n+    ```\n+\n+    Example call: `Clamp(var, 1000, 5000);`\n+\n+  - What to do:\n+\n+    ```c++\n+    template<class T>\n+    inline void Clamp(T* val, const T& min, const T& max)\n+    {\n+        if (*val < min)\n+            *val = min;\n+        else if (*val > max)\n+            *val = max;\n+    }\n+    ```\n+\n+    Example call: `Clamp(&var, 1000, 5000);`\n+\n+- Functions that are part of a class that you do not want to be overridden in inheriting classes should be marked with the `final` specifier.\n+\n+  ```c++\n+  class ClassName : ParentClass\n+  {\n+  public:\n+      virtual void Update() final;\n+  };\n+  ```\n+\n+- Functions that are overridden in an inheriting class that can also be inherited should be marked with the `override` specifier to make it easier to notice which functions belong to the parent class.\n+\n+  ```c++\n+  class ClassName : ParentClass\n+  {\n+  public:\n+      virtual void Update() override;\n+  };\n+  ```\n+\n+### Classes and Structs\n+- Classes and structs that are not intended to be extended through inheritance should be marked with the `final` specifier.\n+\n+  ```c++\n+  class ClassName final","path":"Contributing.md","position":214,"original_position":214,"commit_id":"72ed017164805a2396a812b853f3022df6b2d665","original_commit_id":"72ed017164805a2396a812b853f3022df6b2d665","user":{"login":"neobrain","id":4840017,"avatar_url":"https://avatars.githubusercontent.com/u/4840017?","gravatar_id":"0e2ad4f720c57b96ab4bc6a24c2b59fd","url":"https://api.github.com/users/neobrain","html_url":"https://github.com/neobrain","followers_url":"https://api.github.com/users/neobrain/followers","following_url":"https://api.github.com/users/neobrain/following{/other_user}","gists_url":"https://api.github.com/users/neobrain/gists{/gist_id}","starred_url":"https://api.github.com/users/neobrain/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/neobrain/subscriptions","organizations_url":"https://api.github.com/users/neobrain/orgs","repos_url":"https://api.github.com/users/neobrain/repos","events_url":"https://api.github.com/users/neobrain/events{/privacy}","received_events_url":"https://api.github.com/users/neobrain/received_events","type":"User","site_admin":false},"body":"Maybe make this `class ClassName final : public SomeOtherClassName` just to spare people the effort of looking up where the specifier has to be placed ;)","created_at":"2014-05-25T09:39:39Z","updated_at":"2014-05-25T09:39:39Z","html_url":"https://github.com/dolphin-emu/dolphin/pull/410#discussion_r13030024","pull_request_url":"https://api.github.com/repos/dolphin-emu/dolphin/pulls/410","_links":{"self":{"href":"https://api.github.com/repos/dolphin-emu/dolphin/pulls/comments/13030024"},"html":{"href":"https://github.com/dolphin-emu/dolphin/pull/410#discussion_r13030024"},"pull_request":{"href":"https://api.github.com/repos/dolphin-emu/dolphin/pulls/410"}}}}