1

I am looking to possibly shorten my code. It seems super redundent but I am not sure how else I would be checking for a specific condition to set a variable besides a large IF statement?

IF MaxContaminationCode := -2 THEN
        ContaminationClass := '000';
    ELSIF MaxContaminationCode := -1 THEN
        ContaminationClass := '00';
    ELSIF MaxContaminationCode := 0 THEN
        ContaminationClass := '0';
    ELSIF MaxContaminationCode := 1 THEN
        ContaminationClass := '1';
    ELSIF MaxContaminationCode := 2 THEN
        ContaminationClass := '2';
    ELSIF MaxContaminationCode := 3 THEN
        ContaminationClass := '3';
    ELSIF MaxContaminationCode := 4 THEN
        ContaminationClass := '4';
    ELSIF MaxContaminationCode := 5 THEN
        ContaminationClass := '5';
    ELSIF MaxContaminationCode := 6 THEN
        ContaminationClass := '6';
    ELSIF MaxContaminationCode := 7 THEN
        ContaminationClass := '7';
    ELSIF MaxContaminationCode := 8 THEN
        ContaminationClass := '8';
    ELSIF MaxContaminationCode := 9 THEN
        ContaminationClass := '9';
    ELSIF MaxContaminationCode := 10 THEN
        ContaminationClass := '10';
    ELSIF MaxContaminationCode := 11 THEN
        ContaminationClass := '11';
    ELSIF MaxContaminationCode := 12 THEN
        ContaminationClass := '12';
    END_IF

With this code, I am checking to see if a calculated value (MaxContaminationCode), which is an INT, is a specific value. If it is the specific value set "ContaminationClass" (which is a string), to the respective value.

lt123
  • 117
  • 1
  • 2
  • 8
  • You only need 3 cases: -2 and -1, everything else should be a simple int-to-string conversion – Gereon Oct 23 '20 at 15:15

2 Answers2

1

In general, if you have many different cases, then it is preferable to use the CASE statement ('switch' in other languages) as DrBwts said. Also, as Gereon said, in your example you could decrease the cases you check. So for your example it would look something like this:

CASE MaxContaminationCode OF
-2:
  ContaminationClass := '000';
-1:
  ContaminationClass := '00';
0..12: // if you ONLY want 0 to 12, otherwise use ELSE here
  ContaminationClass := INT_TO_STRING(MaxContaminationCode);
END_CASE
Guiorgy
  • 1,405
  • 9
  • 26
  • Thanks for this suggestion, I knew there had to be some way to clean it up, but wasn't sure what that way logically could be. – lt123 Oct 26 '20 at 10:43
  • Side note: what do you mean by your comment "0..12: // if you ONLY want 0 to 12, otherwise use ELSE here"? – lt123 Oct 26 '20 at 11:24
  • Clarifying my comment: In `CASE` statements you can specify ranges (example `0..12`, any number between 0 and 12). If you want any number besides -1 and -2 to be converted to STRING, use `ELSE` there (that will include other negative numbers too, like -3 etc.). The example I wrote will do the **exact** same thing you code in your question does, meaning it will return an empty string ("") when `MaxContaminationCode` is a negative number other than -1 and -2, and when it's greater that 12. I don't know if this is the behavior you wanted, thus the comment. – Guiorgy Oct 26 '20 at 12:59
1

I agree with suggestion above. But to make it even more compact (-2 lines).

ContaminationClass := INT_TO_STRING(MaxContaminationCode);
IF MaxContaminationCode = -2 THEN
    ContaminationClass := '000';
ELSIF MaxContaminationCode = -1 THEN
    ContaminationClass := '00';
END_IF
Sergey Romanov
  • 2,949
  • 4
  • 23
  • 38
  • If you are going to go there, then you could shorten my code by having the case and instruction on the same line, like this: `-2: ContaminationClass := '000';`, which is how the CODESYS documentation on CASE does in their example (see the link in my answer). FYI, this would make my code 1 line less than yours. I just personally prefer them on separate lines. – Guiorgy Oct 26 '20 at 13:05
  • Hey, I just had to give my 2 cents. I can put `IF` and `THEN` also in one line )) Then my code would be only 3 lines. Look I posted my variant not to humiliate yours, just as a part of community. Do not take it personal :)) – Sergey Romanov Oct 29 '20 at 05:47