Feature/coding standards

[TheMaverickProgrammer]

I used the following chromium resources: style guide do's and dont's

Which are supposed to be an extension on top of Google's C++ style guide. I use this the most: guide

My updates can be summarized as follows:

1. getters can be snake_case of the variable name (hence the trailing underscore in variable names) which looks nice
2. "Major" functions are functions that perform some action on behalf of or for the class. Well-formed major functions must have every first word capitalized
3. boolean queries are not "getters" - they are major functions so things like IsReady() is appropriately formed 3.2 Functions that "GetX" something from a set of inputs were renamed to "FetchX" to avoid "getter" nomenclature. These types of functions are considered major functions since they perform actions unrelated to directly returning member values.
4. Use brace initialization where you absolutely can
5. constants and enums must precede with a lower-case k in the name. This avoids conflicts with C macros
6. Only types with non-trivial de-constructors should be marked static in the global namespace
7. Mark functions noexcept where appropriate
8. API functions can be capitalized as it looks like they are "acting on" or "for" an object by design. This keeps the rest of the codebase looking consistent too.
9. Use const type& when passing values that do not expect a lifetime after the function because this syntax can bind to both lvalues and temporaries
10. Use size_t for counting or returning elements in standard library containers
11. structs can have publicly accessible member values that do not have a trailing underscore as this library was already doing
12. Added a .gitignore file for microsoft visual studio projects

Other things:

1. I fixed some spelling mistakes and translation mistakes in the codebase
2. I const-qualified functions in order to respect and make clear the intention of the function
3. I refrained from exposing pointers to internal shader programs so that they can be safely used
4. Merged with master and made sure all examples run

Bug:

1. Unsure why, but the Text demo with the arrow glyphs (wide char) do not render correctly anymore. Debugger shows blocks instead of arrows. It worked before and I'm unsure which commit broke it. From what I can tell, it shouldn't have been affected like that. Can you take a look at it?

[TheMaverickProgrammer]

I am really confused by most if not all of the rejects as they conflict with the coding standard.

inline is mentioned by the standard, static trivial variables are fine, const qualifiers are mentioned, inline is recommended for getters, k-prefix before enums, brace initialization, etc... these are all mentioned in the links I shared.

[ArthurSonzogni]

Thanks! This is very good! I just wanted to reject/modify the elements below:

inline is mentioned by the standard, static trivial variables are fine, const qualifiers are mentioned, inline is recommended for getters, k-prefix before enums, brace initialization, etc... these are all mentioned in the links I shared.

Yes, sorry ;-) Your patch is huge and I was trying to look at everything one by one and wanted to discuss it later with you.

---

inline is mentioned by the standard.

I am not sure to see what the goal of using inline here. What part of the coding style require/recommend using inline?

I have never used myself them on Chrome.

The compiler should be able to make the right optimizations itself. I don't have to try to do the compiler's job without good reasons.

---

static trivial variable are fine

I am not sure to see what you are referring to here. Are you talking about the global variable? I removed static, because static is often used to make a variable declared inside a non-global scope to behave as if it was global. global + static doesn't really make sense to me. Why do you thing we must make global variable static?

---

const qualifiers are mentioned

I removed 'const' only for returned values. I use 'const' when the value matters to the callee. Those are typically elements owned by the callee and are usually returned as const references / pointers. If the returned elements is a value, then the caller has full ownership of the element. We mustn't prevent the caller from mutating values it owns.

---

I removed the .gitignore, because I don't consider relying on it to be a good practice. Folks must control what they do when adding a file to git. I don't need help from a .gitignore file.

---

brace initialization

• For member with deterministic constructor, I prefer doing nothing.
• For member with in-deterministic constructor (bool, float, int, glm::vec, ...), we must provide a deterministic alternative. I prefer using T t = value; over T t{value} This is what is usually used on Chrome.

This is recommended by the do/don't:

Use C++11 “uniform init” syntax (“{}” without ‘=’) only when neither of the above work:

---

k-prefix before enums

Yes this is true. It would make me very sad adopting the google coding style for Color::Blue. I agree this goes against the coding style.

Maybe I will restore this part later, time for me to grieve and accept it.

[TheMaverickProgrammer]

inline

true compiler optimizations are pretty good but the coding standard said we could use them for one-liner getters. inline is more of a hint these days but as a cynic myself I don't trust everything. I'd personally mark it inline so 1-2 line getter functions skip needing to de-ref the object and inject the assembly directly for marginal speed boosts.

static in global namespace

static keyword in the global namespace has a different lifetime in the C++ runtime than other non-static variables. They are initialzed first and deinitialized last. They share the same space in the symbol table across all files when linking. post C++-11 static variables are also guaranteed to be thread-safe which your audio module is not by removing the static counters. (It is not to begin with because of the global device pointer but I digress we can patch that later)

const

nothing is stopping someone from writing int left = view.Left(); // const int View::Left() const. The value can be copied and mutated. A copy will always happen even without marking const because we're returning a new value. The only gotcha I see for the const argument is if you are using auto e.g. auto left = view.Left(); // const!

.gitignore

ok this sounds like a matter of preference

brace-initialization

I mis-read that section. Thanks for correcting me.

k-prefix grieving

lol I understand

[ArthurSonzogni]

static keyword in the global namespace has a different lifetime in the C++ runtime than other non-static variables. They are initialzed first and deinitialized last. They share the same space in the symbol table across all files when linking. post C++-11 static variables are also guaranteed to be thread-safe which your audio module is not by removing the static counters. (It is not to begin with because of the global device pointer but I digress we can patch that later)

Thanks!

I believe everything should go right, as long as I am using basic types (pointers, bool, int, float) and I am zero initializing them. The global will be put in the .bss section, copied into memory and there won't be any code execution to initialize them.

Ideally, I should remove every global variables, but that's for another issue.

I renamed every global variables to use the g_ prefix.