From @cousteaulecommandant on 2018-02-22

I think that, although the code becomes more compact and simple, it also becomes less explicit and deviates from the typical Arduino code. Keep in mind that this is an example and thus needs to be easy to understand and modify.

• Remove unusedledPin constant

I don't think this is a good idea. Sure, you could just use the macro every time instead of assigning it to a constant, but then if you want to change the pin you use you have to change EVERY line where it appears and risk forgetting one of them; it's better practice to define the pin at the beginning so that it can be modified easily.

• Change if/else statement to demonstrate shorthand variable flipping

I think this is not the point of this example; better to leave it as an explicit "if it's low then set it high, otherwise set it low" and leave the usage of! to another example.
Also I'd sayHIGH andLOW are not to be interpreted as true and false boolean values in Arduino, but rather as two abstract pin states (which just happen to be defined as1 and0, but that's an implementation detail that isn't evenin their documentation). Think of them as anenum.

• Make comments more natural language

This part was good in my opinion (except for theConstants won't change/Variables will change part; I think it's useful to specify that distinction since they're part of what's being explained here).

PS: I forgot to mention, but it seems that you removed the newline at the end of the file. Although C++ allows it and Arduino might be fine with that, it's better practice to always have your text files ending in a newline (for example it's illegal not to do so in C).

From @DRSDavidSoft on 2018-02-22

@codetheorist With the introduction ofLED_BUILTIN, I thought this exampleshould have been modified according to your edit. However, as@cousteaulecommandant already pointed out, that' not the point of the example.

However, as a better coding pattern for amateurs, I'd appreciate if@codetheorist still somehow makes its way to the examples.

From @cousteaulecommandant on 2018-02-22

I disagree. AlthoughLED_BUILTIN is a useful macro for referring to the built-in LED, I'd rather keep it this way, since this macro refers to a specific LED whereas the constant intends to refer to "whichever LED the design uses". It's not really redundant since both refer to different concepts ("LED used in this program" vs "LED built into board").

Personally I strongly believe it's better practice to define all constants that will be used in the application in a single place near the beginning (as constants or as macros, which may or may not refer to other macros). I have found that it eventually simplifies modifying the program later.

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign ourContributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let usrecheck it.}