KorneliusThomas / PIDcontrollersModularProfessional

4 stars 2 forks source link

Code Nonsense #2 #3

Open paulalting opened 4 years ago

paulalting commented 4 years ago

Hello Kornelius,

In you CPID.cpp code you have a goto statement at line 137, yikes!

if (Td==0) // if controller parameter Td time zero - no filtr for D part
     {D=0; variab1=0;
     goto ou;}

//calculate D according to formula pTd(1+pTd)

variab1 = ( diff - DF )*0.1/Td +DF ;
D = ( variab1 - DF )* Td * gain * 10;

ou: DF=variab1 ;

I have never had the need to ever use a goto statement in C or C++ to this day. Could the code be better written as follows, again, my re-factoring of this code to make it more readable and hopefully better:

if (Td == 0.0) {                // if controller parameter Td time zero - no filter for D part
  D = 0.0;
  variab1 = 0.0;
}
else {
  variab1 = (error - DF) * 0.1 / Td + DF;
  D = (variab1 - DF) * Td * gain * 10.0;    // calculate D according to formula pTd(1 + pTd)
}

I think this is a much better way, don't you agree ? Paul Alting van Geusau

KorneliusThomas commented 4 years ago

Hi Paul, yes I know, today men say no goto statement.Your commands equals mein and you can write it in this way.In launguages I used to write goto was used from time totime because is very, very fast. It is in PLC advantage. I think this few goto made the program better readable.

But it is up to you what to do. Kornelius

W czwartek, 27 lutego 2020, 09:25:30 CET, Paul Alting van Geusau <notifications@github.com> napisał(-a):  

Hello Kornelius,

In you CPID.cpp code you have a goto statement at line 137, yikes! if (Td==0) // if controller parameter Td time zero - no filtr for D part {D=0; variab1=0; goto ou;}

//calculate D according to formula pTd(1+pTd)

variab1 = ( diff - DF )0.1/Td +DF ; D = ( variab1 - DF ) Td gain 10;

ou: DF=variab1 ;

I have never had the need to ever use a goto statement in C or C++ to this day. Could the code be better written as follows, again, my re-factoring of this code to make it more readable and hopefully better: if (Td == 0.0) { // if controller parameter Td time zero - no filter for D part D = 0.0; variab1 = 0.0; } else { variab1 = (error - DF) 0.1 / Td + DF; D = (variab1 - DF) Td gain 10.0; // calculate D according to formula pTd(1 + pTd) }

I think this is a much better way, don't you agree ? Paul Alting van Geusau

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

paulalting commented 4 years ago

I think using to goto in this case actually makes the code slower, because each time the condition is true, it will do have to perform a relative jump to get around a bit of code. By having the if else code, it never needs to do a relative jump, making it more programmatically better code in my opinion. I try to imagine the pseudo assembler code that will be generated or how I would write it in assembler code.

A goto statement is never more readable, as you need to find where the goto label is and that there could be one or two or more and they might be before the goto or after the goto statement, meaning you need to search the code.

By rewriting this bit of code to replace the got with the if else is consistent and standard and fast, the compiler knows exactly what is going on.

When I need to make code run faster, I use integer math and then divide out to engineering data on the outside of functions when and only if needed. Floating point math is not always a strong point in these small 8 bit controllers. This is what I do with my simple but fast PID controller.

KorneliusThomas commented 4 years ago

Hi Paul, now I need to go and will be back in few hours,probably at 15 00.Now we have 11 45 in Germany. Kornelius

W czwartek, 27 lutego 2020, 11:11:41 CET, Paul Alting van Geusau <notifications@github.com> napisał(-a):  

I think using to goto in this case actually makes the code slower, because each time the condition is true, it will do have to perform a relative jump to get around a bit of code. By having the if else code, it never needs to do a relative jump, making it more programmatically better code in my opinion. I try to imagine the pseudo assembler code that will be generated or how I would write it in assembler code.

A goto statement is never more readable, as you need to find where the goto label is and that there could be one or two or more and they might be before the goto or after the goto statement, meaning you need to search the code.

By rewriting this bit of code to replace the got with the if else is consistent and standard and fast, the compiler knows exactly what is going on.

When I need to make code run faster, I use integer math and then divide out to engineering data on the outside of functions when and only if needed. Floating point math is not always a strong point in these small 8 bit controllers. This is what I do with my simple but fast PID controller.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub, or unsubscribe.