AngeloCasi / FUGU-ARDUINO-MPPT-FIRMWARE

An open source Arduino ESP32 MPPT Charger firmware equipped with charging algorithms, WiFi, LCD menus & more!
Creative Commons Zero v1.0 Universal
519 stars 133 forks source link

Code readability improvements #22

Open lienbacher opened 2 years ago

lienbacher commented 2 years ago

Hello! I just wanted to note that - while I really notice and appreciate the effort you spent to make your code easy to understand by adding a comment to virtually every single line - it is common practice to focus on having your code easily readable, as opposed to having to explain your code in comments. Good code should not need comments to explain what it does.

With the formatting you chose you actually need to condense your code lines in order to have them properly explained in the same line. As an example, please evaluate the readability of this line:

if (ERR>0||chargingPause==1){buck_Disable();}           //ERROR PRESENT  - Turn off MPPT buck when error is present or when chargingPause is used for a pause override

in opposition to:

if (ERR > 0 || chargingPause == 1) {
    buck_Disable();  
}

I think the line expanded to 3 lines with proper spaces absolutely does not need any comment to explain what's going on and is rather easy to understand in one go while overflying the code to learn how it works.

I find it is rather exhausting and difficult to visually decode condensed lines, especially if the same occurs line after line. If you look at other peoples code you will find the expanded writing style is significantly more common.

TBAMax commented 2 years ago

I just comment here for the encouragement. I find the currently used style with lot of comments quite good. This gives a chance to choose: read the code and try to understand what is going on, or read the comments and get the quick impression of the intent of programmer. At first it is always unclear what variables are for what, and added comments speed up the understanding of the code quite a lot. After, when already familiar with the structure of the program and structure of the variables, the comments may seem unnessesary of course. And for me for example it makes no difference if the condensed style of expanded style is used, I would still have to make a search of the used variables to get the clear idea what those are used for.

lienbacher commented 2 years ago

I respect that you find it good to gave a choice between reading comments versus reading code. However please understand that good code does not need comments to explain what it does. Good variable naming makes it very clear what purpose the variable serves and should not need a comment to explain it. I‘m sorry, but it‘s just a very bad way of writing code.

TBAMax commented 2 years ago

@lienbacher I disagree in that. I rather read that, than some hyper arrogant programmers code who thinks his/hers code is so good it do not need any comments. Yes there are some things that could be better, there always are: for example PWM floor could very well be called PWMfloor instead of I do not remember what...

lienbacher commented 2 years ago

I am neither a programmer (I only occasionally write code in my spare time), nor do I think my code is so good it does not need comments, which kinda rules out me being a hyper arrogant programmer. I just have a thing for reading good code and have spent a fair bit of research about what is considered well written code by people who live and breathe coding.

I also realize I may have my words too strictly, sorry for that. When I said "Good code should not need comments to explain what it does." I did NOT mean "Comments are bad and you must avoid using them in your code at all cost". But compressing your code into single lines to you can fit a comment into every single line is just way over the top.

You may disagree, and I am not going to call you anything for that.