Skip to content
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

Coding Style Standards #36

Open
mdclyburn opened this issue Nov 1, 2015 · 20 comments
Open

Coding Style Standards #36

mdclyburn opened this issue Nov 1, 2015 · 20 comments
Assignees
Milestone

Comments

@mdclyburn
Copy link
Contributor

This isn't a project on the scale of something like Linux or *BSD, but we should have some standard of coding style enforced on submitted code (camel-case, curly brace placement, tabs vs. spaces). The LaTeX should probably have some standard as well. Something more than what is given in the README.

@robertu94
Copy link
Contributor

Thanks for volunteering!

I think the best way to handle this would be to enforce one of the styles that can be enforced via gnu indent. There are 3 sane defaults for indent : kr, gnu, and linux. Of these, I personally like the look of the linux indent style

@robertu94
Copy link
Contributor

I am sure that @ProtractorNinja has an opinion on this.

@robertu94
Copy link
Contributor

As for latex, I don't think that there is an equivalent tool unless we write one.

@mdclyburn
Copy link
Contributor Author

OK...

@ProtractorNinja
Copy link
Contributor

Err... I don't think we need to voluntell assign anyone to this quite yet. It's just a discussion.

I am against using indent (or any other automated tool) as any kind of foundation for a coding standard, as it does not address a great many important stylistic points. One of them that Marshall already mentioned is variable/function naming style.

Instead, I think that we should adopt Google's C++ Style Guide, as it is well known, already complete, and easy to introduce. There are other style guides for other languages, including Python and Bash, but not LaTeX.

Of course, LaTeX hardly needs a style guide to the degree that C++ does, so it may suffice to just enforce the LaTeX-relevant bits from Google's guide, like indentation and custom naming conventions. Or, even simpler, this Stack Exchange question suggests basing the code on one piece of advice: "Use whitespace to make your code readable."

@robertu94
Copy link
Contributor

I completely disagree about the use of automated tooling to enforce coding conventions. Enforcing coding conventions is hard enough if you remember them (forgetting, code that doesn't read clearly following the standards, ease of accidentally messing it up). When the code base becomes sufficiently large this can become something of a pain without automated tools. I also think that using an automated tool can assist with someone who is just entering the project and wants a lower barrier to entry. For experienced developers it saves time.

As for indent specifically, I know that it doesn't enforce code style with regards to variable/function names or comments. However, it is installed on the lab machines here, and does much more than something that we would have the time/desire to write ourselves. It would be a good starting point.

I think that the Google C++ style guide covers several things that are quite excessive for contest programming environments. For example, the sections entitled Namespace, static/globals, and access control simply don't make sense in competition environments. While the Google C++ guide is optimizing for reading, we are (at least trying to) optimize for typing. I am not sure that their style guide is appropriate.

I agree regarding the LaTeX style with the exception of trying to apply Google's guide to LaTeX. We may be able to write a quick perl/python script that does what we want with regards to indentation, or we could just rely on vim's '=' operator, which does a decent job(provided that white space options have been set).

@robertu94 robertu94 modified the milestone: NEXT Nov 1, 2015
@mdclyburn
Copy link
Contributor Author

Austin does bring up a good point about there being more to coding style than just indentation. Even if we don't have tools to do everything for us, I don't see it being a big problem to write them to at least check for us (or maybe I haven't finished generating the possibilities in my head).

As for the LaTeX part, I want to see rules regarding indentation (of course), graphics importing, indexing, etc. Another thing that I would like to see outlined is how we lay out our sections, what is mandatory for each section, and what goes where among other things. I'm currently working on the segment tree and it's currently under the 'Basic Data Structures' section. This will have to change, but what defines a 'basic data structure'? One that is included in the STL? Properly made arbitrary judgement?

I don't think it would be much of a problem to author our own style guide to include in the repository. We could just collaboratively list the things and discuss how we want them handled. This is clearly something that we must deliberate on for a bit. So how about this? I will find some time to draft up a list of topics we want to consider and we can start there perhaps?

@robertu94
Copy link
Contributor

I also agree with Austin's point that there is more to coding style than indentation, I just think that using a tooling to help enforce even a portion of the style guide is better than none at all.

"Basic Data Structures" is kind of a deceptive name. We have graphs there, and they are not trivial. We should probably put the segment tree there also. The section should probably be renamed "Data structures."

I agree @mdclyburn, a rough draft would be a good place to start. I would put it in CONTRIBUTING.md and reference it in the README.

@ProtractorNinja
Copy link
Contributor

Since we're likely to discuss both LaTeX organization and code styling rather extensively, why don't we split the LaTeX discussion into a separate issue to better encapsulate both topics? Marshall's points made it obvious that there's actually a lot to cover in terms of document organization.

Let me first clarify that I think using a checker tool of some sort would be awesome, but only after we've decided what it is that we want to check for. I don't like the idea of indent being the foundation of our style guide. It would be an easy basis to start from, but from that point we would be unable to evolve past what indent enforces, and we would still need yet another tool to address what indent does not. Further, indent is a destructive auto-formatter, not a linter, and leaves little room for common-sense intervention except through ugly preprocessor definitions. indent is not an appropriate tool for this situation.

With that said, I don't quite understand the downsides of using Google's Style Guide -- or at least using it as a foundation of our own guide.

  • Yes, the Google guide's scope is wider than ours. Why is that a problem? It would be much easier to compile a list of sections that Hack Pack submissions should ideally adhere to than to create our own full guide from scratch. It's organized remarkably well, after all, and every section is easy to link to. Authors can very easily ignore any irrelevant sections.
  • Yes, the Google guide is long... because there are so many individual sections. By itself, each section is concise, concrete, and well-written. Anyone we expose this guide to is going to discover a lot of ways to improve their code.
  • Yes, the Google guide emphasizes readability over conciseness. The concrete downsides of this are... what, exactly? The Hack Pack is both a quick resource and a learning reference. Tidy variable naming conventions and function organization will significantly aid comprehension for anyone trying to learn, and our competition coders are bright enough to adjust lengthy names when typing out an algorithm. Clean, understandable code also means that the underlying structure will be easier to amend in order to solve a slightly different problem. The negative impact of slightly more verbose code chunks in the packet is negligible compared to all the other things in the packet that we could slim down.

Meanwhile, I certainly understand the advantages of composing a new guide from scratch -- we're able to pick out what's important to us, and we're allowed to feel independent. But someone else has already done this work for us. Adding our own style guide is one more obstacle in the way of generating useful content for the Hack Pack. There will be things left out, things important to one person but to nobody else, and things we can't agree on. Are there any real benefits to doing so when a simple solution already exists -- picking out the relevant bits from Google's guide and going from there?

As for automated tools -- that's another thing that's already been implemented almost in full. cpplint.py is a non-destructive linter that checks for compliance with Google's C++ style guide. It's cross-platform, won't disrupt anyone's edge case alignment, can be overridden, and most importantly, is already done.

@mdclyburn
Copy link
Contributor Author

'tis done. Issue #38 is for LaTeX.

@mdclyburn
Copy link
Contributor Author

@ProtractorNinja That actually sounds like a good idea. It's always nice to take advantage of others' work. That's what it's there for. I'll look at it sometime soon and get back to say how I feel about it.

@robertu94
Copy link
Contributor

Two standards that we may want to consider would be the Linux and FreeBSD kernel developer style guides. Both of these style are more concise than the Google style guide and don't use camel or title case which is just not ascetically pleasing.

@ProtractorNinja
Copy link
Contributor

@robertu94, can you summarize some of the pros/cons of either of those that you mentioned?

@robertu94
Copy link
Contributor

All style standards have roughly the same goal in mind, to improve readability. These standards accomplish this without the poor case choices and unnecessary use of name-spaces/conventions that lengthen code to improve reliability for large enterprise deployments that a standard like Google's would provide.

@ProtractorNinja
Copy link
Contributor

Looks like I asked the wrong question. For the Linux, FreeBSD, and Google guidelines, could you separately itemize some of the pros and cons of each? Breaking each guide down in such a way will help us to figure out which one fits our needs the best, and should also help succinctly clarify why the Google style guide is probably not the best choice.

@robertu94
Copy link
Contributor

Google

Pros:

  • Functions - parameter order, short functions, reference arguments, function
    overloading, default arguements, trailing return types all still make
    sense in a contest enviorment.
  • Naming - Type Names, variable names, enum names, macro names,
  • Comment Style - class comments, functions comments, variables comments, every
    thing else in comments
  • Formatting - Line length, non-ascii characters, conditionals, loops and
    switch, points booleans, returns, variable initialization, preprocessor
    directors, classes, namespaces, vertical whitespace
  • Ease of Use - The style guide is very easy to read and find examples
  • Tooling - cpplint.py non-destructive linter exists

Cons:

  • Header files - completely irrelevant for contest enviroments
  • Scoping - Advise doesn't make any sense for contest environments
  • Classes - While OO is great for encapsulation it does make sense for contests.
    The only item from this section that makes sense is perhaps declaration
    order.
  • Google Specific Magic - The name says it all; Not for contest environments
  • Other C++ Features - With the exception of tempesting, not sure this makes
    sense
  • Naming - general rules make sense except for names like i,j,k, and n used in
    proper context. file_names are specified in contests. trailing underscores
    are gross, globals are discouraged, Mixed case function names, namespace
    names,
  • Advice against macros
  • Comment Style - license boilerplate, file contents are unnecessary because they
    are in other files in the hackpack
  • Formatting - spaces (who uses 2 spaces??), weird function formatting
  • Tooling - cpplint.py does not use a C parser on the backend

Linux

Pros:

  • Tabs for indentation
  • 80 columns limit
  • bracing style makes sense without unnecessary indentation
  • Naming convention make more sense for contest environments
  • Function guidelines improve readability
  • Simple commenting formating guidelines
  • Don't reinvent the stl
  • Avoid editor modelines
  • Ease of Use - Very short and easy to read
  • Code looks ascetically pleasing
  • Tooling - indent is a destructive formatter handles all whitespace problems.

Cons:

  • typedef rules improve readability but also significantly lengthen lines and
    hurt readability
  • recommends use of goto which makes sense in the kernel due to lack of
    exception handling, but often can cause fraken-bugs.
  • comments on assembly, allocating memory, and kernel messages do not apply

BSD

Pros:

  • Very similar to Linux and KNR style guide
  • Code looks ascetically pleasing

Cons:

  • Several sections are specific to the division of the base system from ports.
  • Ease of Use - Several of the rules are implicitly written.
  • No linter that I found

@ProtractorNinja
Copy link
Contributor

Hmm.. after looking at all these options again and coming back to what we really started this conversation for, I think we're falling victim to scope creep.

All the style guides cover more than we really need, and it's been made clear that isolating the relevant bits of any of them won't cut it. It seems to me that we're not going to need to clarify much more than (my opinions in parentheses):

  • Whitespace (use tabs)
  • Line width (80 characters, please)
  • Brace style (K&R)
  • Style and naming conventions for..
    • Variables (snake_case)
    • Typedefs (snake_case)
    • Constants (UPPER_CASE)
    • Functions (snake_case)
    • With a note on preferred name verbosity (clarity overrules conciseness, I think)
  • namespace scoping (just do using namespace std;)
  • File names
  • Comments ( I like using C99-style "//"s for everything )
  • Other core stylistic bits

Addressing these topics will resolve the majority of inconsistencies without us having to deal with blotting out the extras.

@robertu94
Copy link
Contributor

While I agree with the kernel of the style guide defined in the previous comment, I still think that it would be beneficial to adopt an external style guide. I think that the external style guides:

  1. Cover all of the issues that we care about.
  2. Optimize for our time and effort to benefit ratio. I believe that these standards have been more clearly written and extensive than we could hope to achieve independently.
  3. Avoid the creation of unnecessary standards.
  4. Allow for us to use existing tooling developed for the external standards.

I do not advocate for the Google Style guide because I feel that we would have to very extensively strike sections and rewrite large parts of sections to be in line with the goals that we have (conciseness, etc).
At the point that we have completed this editing, it would be more efficient to write our own guide.

I personally think that the Linux Kernel Developer Style guide is the most aligned with our goals with the exception of sections 10 (Kconfigs), 13 (Kernel Messages), 14 (Memory Allocations in the Kernel), 17(Kernel Macros), and 19 (Inline assembly) which could be stricken in their entirety with out editing sections as they are entirely specific to kernel development and noticeable as such.

@robertu94 robertu94 modified the milestones: NEXT, Spring 2016 Nov 14, 2015
@robertu94
Copy link
Contributor

I will go ahead and write a rough draft of this so that we have something a bit more concrete to review.

@robertu94 robertu94 self-assigned this Nov 17, 2015
@ProtractorNinja
Copy link
Contributor

( see #61 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants