mrrwa / NmraDcc

NMRA Digital Command Control (DCC) Library
GNU Lesser General Public License v2.1
135 stars 53 forks source link

Namespace cleanup for NmraDcc class/functions #39

Closed davidzuhn closed 4 years ago

davidzuhn commented 4 years ago

This is a big PR with several "big" changes in terms of what's touched in the source, although very few things are actually changed substantively.

First of all, a set of patches to make the source more consistent (one line ending, one indent length, one brace style). I'll undo these if I must, but I spent too much time on one fix already where I missed that the if statement didn't have a block following but a bare statement.

Next, I make the ExternalInterruptHandler a static function. Why? Because my code already had a function called ExternalInterruptHandler (which itself wasn't static), and there's no reason the NmraDcc code needs to pollute the namespace like that.

Then I added almost all of the functions to the NmraDcc class (as private methods). Because ackCV and ackAdvancedCV get used via function pointers, that will take a little more work to move them into the class namespace.

This sets everything up for the next PR, which will be the code that makes EEPROM an optional (rather than required) component of the NmraDcc library.

davidzuhn commented 4 years ago

I'm perfectly amenable to any consistent formatting style. Line widths as well.

As for your Linux box, I'm surprised it doesn't use gnu indent by default (my raspberry pi has GNU indent 2.2.12 installed as 'indent' and that's the only Linux I have immediate access to). On my Mac, I use the Homebrew system for a lot of my tool installation, and there the package is installed as gnuindent as to not interfere with the Apple provided indent (which isn't gnu).

Lastly, I've been involved in projects that tried to indent on every commit, and that caused more problems that not. However, putting in a commit check that examines that indent style and fails on a variation seemed to work much better. Personally, I don't like any changes to the code between the human typing "git commit" and the eventual version in the repository. I'm not even a fan of $Id$ type keyword expansions.

davidzuhn commented 4 years ago

I've moved the formatting changes to a separate PR, and will close this one. Once the formatting is to your liking, I'll create yet another PR with the substantive code changes.