Skip to content

Standardize on style #18

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

Closed
strega-nil opened this issue Sep 19, 2017 · 8 comments
Closed

Standardize on style #18

strega-nil opened this issue Sep 19, 2017 · 8 comments
Labels
Discussion This is open for a discussion. General

Comments

@strega-nil
Copy link

No description provided.

@leios
Copy link
Member

leios commented Sep 19, 2017

Hey ubsan. This is a discussion we definitely need to have. I sent you a twitter message asking if you would join the discord channel discussion (if only for a brief bit) because we had a pretty lengthy discussion on this topic and your PR's. I guess I should have sent you an e-mail instead, sorry about that!

@leios
Copy link
Member

leios commented Sep 19, 2017

But I guess it's fine to have the convo here. As far as style is concerned, I can only really speak to the languages I know (C / C++, Fortran, Julia, and Python). To me, readability is most important, but I also think the code should compile as-is (if possible). There will come a time when it is too difficult to do this, simply because the algorithms require too much code, but we can deal with that when the time comes.

Here's what makes code readable (to me):

  1. Comments. I really like the comment block at the start of my files because it gives an indication of what the file is supposed to do and how it will do that along with basic compilation instructions. The simpler, the better. That said, I think having the same comment block at the start of every file does not make sense if the code is pasted at the end of the articles (as they currently are) -- SEE NEXT COMMENT
  2. Explicit typing whenever possible to avoid ambiguity, also if choosing between types, choose the simpler type. Basically, if you are reading the code as an outsider to the language, it should make sense.
  3. Variable names should be simple and clear. I don't care about the casing so much. This is super dependent on what people are used to and really doesn't matter for this.
  4. Little visual clutter. If my eyes bug out when looking at the code, it is poorly written.
  5. ~"80-column rule". This one isn't so important, but because of how the code is displayed in text, it better look good in an editor with a relatively small width. In general, I feel that this helps keep the code more readable anyway, but that's up to debate.

Because of the nature of the book, it's alright for people to have different styles, but the cleanest implementation will be accepted. When reading the code, I should never have to ask the following questions:

  1. "What type this?"
  2. "What is this variable?"
  3. "What is the author trying to do?"

If these questions come up to the reader, there better be a comment nearby justifying why it is unclear. And if people submit versions of the code that are different, they should justify why their code is better and should be used instead of the code already present.

I fully intend to have a code/ directory in all the articles and want to make that change this week, but I haven't yet had the time. There's a lot of stuff I need to get done to account for all the contributors. We should hammer out a list of guidelines for contributers from here on so that we are clear.

Please let me know your thoughts. Algorithms are (mostly) language independent. The reader should be able grab an implementation in their favorite language, immediately understand it, and then play with it on their own to understand how it works.

@leios
Copy link
Member

leios commented Sep 19, 2017

Alright, after a bit of internal discussion, I think the comment block is unnecessary in this case. Everyone knows the purpose of the code (because it's in the above article) and compile instructions are only really complicated if you want to avoid making makefiles. If we have a good chapter on all the different languages, then compilation should be straightforward.

Also: Comments in general should probably be used mainly to answer language-specific ambiguities. The rest of the questions should be answered in the accompanying article.

I think this can be summed up as: Follow the style guide for the language you are writing in, but remember that there will be newbies reading, so keep it simple.

@strega-nil
Copy link
Author

strega-nil commented Sep 19, 2017

So, as someone who writes a lot of C++, I'd like to base the style on these two things:

https://herbsutter.com/2013/08/12/gotw-94-solution-aaa-style-almost-always-auto/

https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md

The latter's formatting is not my favorite, but I think it's reasonable enough, and has support of the committee, and is, I think, a good base.

Some highlights (with my own spin where the guidelines are inconsistent):

  • 4 space indentation (no tabs)

  • K&R style blocks

int main()
{
    if (foo) {
        bar();
        baz();
    }
}
  • if the statement that's attached to an if or while or for is one statement, and you can fit the statement plus the if on one line (and the line is pretty short), you can use blockless style:
if (x) return; // okay
if (y && z && x)
    do_something_complicated(); // no
  • if there is an else, do not use blockless style. Use hugging braces.
// no
if (x)
    return;
else
    do_something_else();
// yes
if (x) {
    return;
} else {
    do_something_else();
}
  • use auto where it leads to prettier code
// use `auto` here
auto x = get_list_of_args();
std::vector<std::string> x = get_list_of_args();
// don't use `auto` here
auto x = static_cast<char*>(nullptr);
char* x = nullptr;
  • do not use C-style casts. Use static_cast or function-style cast exclusively - I assume that there will be no need for either const_cast or reinterpret_cast.

  • use ranged for when possible:

std::vector<int> v = ...;
// yes
for (auto x: v) {
  // use x here
}
// no
for (size_t i = 0; i < v.length(); ++i) {
  // use v[i] here
}
// *really* no
for (int i = 0; i < v.length(); ++i) {
  // use v[i] here
}
  • your code should compile under g++ -std=c++11 -Werror -Wall -Wextra -pedantic as well as clang++ -std=c++11 -Werror -Wall -Wextra -pedantic

  • naming style is as follows:

Types_use_upper_snake_case (includes classes, enums, concepts, etc.)
values_use_lower_snake_case (includes functions, locals, statics, class members, etc.)
MACROS_USE_SCREAMING_SNAKES
  • pointer style is attached to the type, not the identifier
int main(int argc, char** argv); // yes
int main(int argc, char **argv); // no
  • const style is to the left
int foo(const int* x, const int* const* y);
// not
int foo(int const* x, int const* const* y);

(I'm not happy about this one, but that's how the C++ core guidelines and the standard write their code)

  • inline extern static thread_local constexpr is the ordering one should use for the storage class specifiers

  • const volatile is the ordering one should use for the cv-qualifiers.

  • space between ) and {, and in between expressions and binary operators.

  • no space in between a unary operator and the expression.

  • use parens where associativity might be confusing

a + b
a + (b * c) + d
*x + (*y * z)
if (x) {
  ...
}
  • do not using namespace std;

  • use the smallest scope possible.

@strega-nil
Copy link
Author

Side note, on comments:

imo, the first thing in order of importance is accessibility. this means:

  • single line comments are used, at most, for two lines

  • block comments are a simple /* ... */ - there should be no other "comment markers"

this looks like:

// I'm a short comment
// I get dragged out to the next line
int foo();

/*
    I'm a long comment.
    boop ba doop
    balkjdafiosj
*/
int bar();

@strega-nil
Copy link
Author

Alright, we've agreed to use stroustrup style, with auto used rarely or not at all.

#include <iostream>
#include <cmath>

// Euclidean algorithm using modulus
int euclid_mod(int a, int b)
{
    a = std::abs(a);
    b = std::abs(b);
    while (b != 0) {
        auto temp = b;
        b = a%b;
        a = temp;
    }

    return a;
}

// Euclidean algorithm with subtraction
int euclid_sub(int a, int b)
{
    a = std::abs(a);
    b = std::abs(b);
    while (a != b) {
        if (a > b) {
            a -= b;
        }
        else {
            b -= a;
        }
    }

    return a;
}

int main()
{
    int check1 = euclid_mod(64*67, 64*81);
    int check2 = euclid_sub(128*12, 128*77);

    std::cout << check1 << '\n';
    std::cout << check2 << '\n';
}

@leios
Copy link
Member

leios commented Sep 19, 2017

Yeah, I think there are times when auto makes sense, but should be used sparingly in this case. Thanks for the discussion. I'll mention this when writing the "how to contribute" section soon.

Also: Leaving this discussion open in case further discussion is needed down the line.

@leios leios mentioned this issue Feb 19, 2018
15 tasks
@Butt4cak3 Butt4cak3 added the Discussion This is open for a discussion. label May 1, 2018
@Butt4cak3
Copy link
Contributor

I'll just take the initiative and close this one. There hasn't been any activity for quite some time now and I think everything here has been resolved. If not, then feel free to reopen the issue or create a new one.

Gorzoid added a commit to Gorzoid/algorithm-archive that referenced this issue Jul 20, 2018
Gorzoid added a commit to Gorzoid/algorithm-archive that referenced this issue Jul 20, 2018
leios pushed a commit that referenced this issue Aug 3, 2018
* Added cpp file

* Added Gorzoid to CONTRIBUTORS.md

* Fixed style to conform with guidelines #18

* Removed unnecessary define

* Added entry to jarvis_march.md

* Added check for empty array edge case. Changed Iterators to single vector reference

* Changed to resolve edge case of duplicate points.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion This is open for a discussion. General
Projects
None yet
Development

No branches or pull requests

4 participants