JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.71k stars 5.49k forks source link

Taking code style and formatting seriously #47052

Open Moelf opened 2 years ago

Moelf commented 2 years ago

Right now there are many pretty random choices in the code base:

  1. return or not return https://github.com/JuliaLang/julia/blob/f056dbc78172eb1aec1a3b41a4f9b15d3683306e/stdlib/Distributed/src/remotecall.jl#L404-L407 https://github.com/JuliaLang/julia/blob/f056dbc78172eb1aec1a3b41a4f9b15d3683306e/stdlib/Distributed/src/remotecall.jl#L449-L452

  2. space between binary operator? https://github.com/JuliaLang/julia/blob/f056dbc78172eb1aec1a3b41a4f9b15d3683306e/base/bitarray.jl#L126 https://github.com/JuliaLang/julia/blob/f056dbc78172eb1aec1a3b41a4f9b15d3683306e/base/bitarray.jl#L132 https://github.com/JuliaLang/julia/blob/f056dbc78172eb1aec1a3b41a4f9b15d3683306e/base/bitarray.jl#L320

  3. comma and space after element in array literal? https://github.com/JuliaLang/julia/blob/f056dbc78172eb1aec1a3b41a4f9b15d3683306e/test/bitarray.jl#L1631 https://github.com/JuliaLang/julia/blob/f056dbc78172eb1aec1a3b41a4f9b15d3683306e/test/bitarray.jl#L785

proposal / question:

udohjeremiah commented 2 years ago

Will this style guide change be evident in the current style guide recommended in the docs?

Moelf commented 2 years ago

right, at least we can start by actually enforcing the style guide from the docs as a baseline...

rikhuijzer commented 2 years ago

I just spotted a counterexample for the importance of code style and formatting. Take a look at pnt.c at R's base language. Many parts of the code were written about 23 years ago according to Git's history and heck it even contains a goto statement which programmers nowadays agree on is a bad idea (see Dijkstra's famous paper about goto statements).

Note that the code is littered with arbitrary comments, spaces after comma's and no spaces after comma's, the variable names appear mostly meaningless and oh yes, they also use macro's:

    if (df <= 0.0) ML_WARN_return_NAN;

This is my favourite part:

    return pnorm((double)(tt*(1. - s)), del,
             sqrt((double) (1. + tt*tt*2.*s)),
             lower_tail != negdel, log_p);

It is unclear here what part is what argument and note that negdel is false (uhm integer 0) for t >= 0.0 and true (integer 1) otherwise; to add to the readability.

So who cares about this arbitrary part of code deep down in the R base library, right?

Most programmers who use stats have most likely executed code based on this file. It is the basis for the pt function in R which returns the cumulative density at some point x. Even Distributions.jl wrap this C code for the logic for the non-centered t-distribution via Rmath-julia. Other languages probably wrap the logic too and without problems it seems.

This example shows that badly formatted code doesn't imply bad code.

Moelf commented 2 years ago

This example shows that badly formatted code doesn't imply bad code.

who said badly formatted code implies bad code?

udohjeremiah commented 2 years ago

@rikhuijzer you're absolutely right. But IMHO, if a software will thrive on maintenance from people who weren't there when the code was actually first thought of and written, THEN BADLY FORMATTED CODE IMPLIES BAD CODE (in the long term for maintenance).

Formatting code to make it readable (so it can be actively maintained) does not cost a nickel, so I don't see a VALID PROGRAMMING REASON why anyone would opt in for "badly-formatted-but-good-code".

Julia's syntax is super easy to grasp, and if coding style and formatting is taken seriously, the language can be close to the Mathematics and English Language we write in our daily lives. That for me is A GOOD THING!!!

mgkuhn commented 2 years ago

Whether one should use spaces around operators and after commas (or not) is often a question of how much space is available. The Code Formatting Guidelines in CONTRIBUTING.md say

  • use whitespace to make the code more readable
  • try to adhere to a 92 character line length limit

and sometimes that is much easier to achieve in a readable way by dropping the spaces around some high-precedence operators or punctuation, rather than to split a line. Programmer discretion can often lead to far nicer results than automated enforcement, especially concerning horizontal space and alignment.

udohjeremiah commented 2 years ago

Style guides are "recommendations" not "enforcements". It's better to have a "recommendation", than not having one.

Programmer discretion should be chosen over "recommendations", but as you've said "programmer discretion", not "organisation discretion".

IMHO, any code that needs to be read by more than YOU, should follow a standard (when possible).

Moelf commented 2 years ago

the language does not enforce style on anyone's code, this is for style of Julia's own source code.