Re: Code formatting discussion/proposal

On Mon, Jul 6, 2015 at 1:26 PM, Chris Travers <..hidden..> wrote:

Two brief comments....

On Mon, Jul 6, 2015 at 3:04 AM, Erik Huelsmann <..hidden..> wrote:
Hi all,

I'm going through our code quite randomly at the moment while investigating the multi-currency changes.

What stands out at me is that we have:

* different indenting methods (tabs vs spaces)
* different intenting styles (2, 3, 4+ spaces)
* different widths: 80 on most files, but I've seen anything above too
* single vs double underscores as separators in tokens
* etc

Except for the double underscore problem, this is mostly code we inherited from SQL-Ledger. my sense is that our own code is cleaner.

Tabs vs spaces though are an issue in the development of 1.0-1.2 when we settled on 4-space per indent except when that makes it hard to read.

I'm sorry, but it's starting to disturb me (quite a bit). So, here's my proposal:

* We remove all tabs from the code base in one huge commit (except for Makefile)

+1

* From there on, we watch out not to add new tabs to the code base (add a test case?)
* We choose a single intenting style
- and create .emacs and vimrc examples for peolpe to use with our project; publish those on ledgersmb.org

Except for old code, I think we should settle on 80 characters wide and 4 spaces for indent. With some of the SQL-Ledger code that is not practical though. Same for the payments and maybe the contacts code.

* We settle on either 80 or 132 character-width per file and reformat all files which are over the chosen maximum;
--> With emacs starting up at 80 characters width, I'm all for 80 characters (that way multiple windows fit next to each other on my screen)

I would vote for 80. 132 leads to long lines. And with newer code long lines shouldn't really be needed.

* We settle on a standard for using double underscores. My proposal:
- In Perl, we always use a single underscore, as modules are enforced through OO
- in PostgreSQL/pgPLSQL, we use the format <module>__<module-local-identifier>; making the payments code look like payment__list_all_open_invoices() or payment__post().

+1, also with the move to prefixes and PGObject that will become a lot easier to enforce.

(Currently the former exists, but the latter is called payment_post?!)
* Each indenting level takes 2 spaces (to be added to the .emacs and vimrc examples as well)

2 spaces is sometimes needed for old code, but new code as long as it is well structured should be fine with 4. I would say we should settle on 4, and treat anything less as a warning sign.

* SQL keywords in UPPERCASE
+1

* non-sql keywords in lowercase
* COMMENTS whereever in the code base as normal sentences: One capital and then lowercase until the dot.
* Comments are not essays: "Let's ...." and "We ..." are to be avoided;
Example from the payments code: "We want to set a gl_description"
Should be written as: "Set the gl_description"

Any lessons one can take from the payments code are in the "don't do that" category. That section needs to be rewritten (both interfaces).

Thats actually a good example but I would take it further. Comments that have to describe what the code does are big red warning flags. I am hoping that when we get the contacts section moved to JS and Dojo we will get rid of a lot of the rest of the similar comments.

Most of those comments, btw, come from programmers writing down the algorithm in comments before they start and then implementing the comments in code. Thus those comments are of no real help to anyone else. Let's avoid doing that.

I don't have a problem btw with comments as essays. IMHO, comments should describe things which are outside the code -- why's and wherefore's, promises to developers, reasoning, etc. # we need to ___ because ___ is a good comment. # let's ___ is a bad one. # set the gl_description is bad.

The only reason I haven't rewritten the payment code (both interfaces) is because it is complicated by the db design we inherited and I haven't gone into that.

* Further formatting choices and minimum (function) documentation requirements to be established as we go and to be collected on ledgersmb.org.
* Various project-root level files are to point to the ledgersmb.org URL where these docs can be found

Comments?

--
Bye,

Erik.

http://efficito.com -- Hosted accounting and ERP.
Robust and Flexible. No vendor lock-in.

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
Ledger-smb-devel mailing list
..hidden..
https://lists.sourceforge.net/lists/listinfo/ledger-smb-devel

--
Best Wishes,
Chris Travers

Efficito: Hosted Accounting and ERP. Robust and Flexible. No vendor lock-in.
http://www.efficito.com/learn_more

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
Ledger-smb-devel mailing list
..hidden..
https://lists.sourceforge.net/lists/listinfo/ledger-smb-devel

Bye,

Erik.

http://efficito.com -- Hosted accounting and ERP.

Robust and Flexible. No vendor lock-in.