NR Linter - [msg] Unreachable code

General
1

I am going through my NR code trying to clear the Linter error messages. I get this error with all of some similarly coded Nodes, they all do what is expected of them (control MQTT Outputs). Could someone explain to me why this error is displayed and how to get rid of it (other than ignore). Or do I need to ignore as it is a correct message because Linter cannot trap the error correctly?

TIA
Colin J

Code for the Node...

[{"id":"6e8f1e99bbbf3993","type":"function","z":"30b349c08c491058","name":"Diverter V/V","func":"//let boilerDivV = global.get(\"boilerDivV\");\nlet Sonoff1 = \"cmnd/DivV/CH_on/POWER\";\nlet Sonoff2 = \"cmnd/DivV/HW_OK/POWER\";\n\n/*\n        No Power - B - 0\n        White - AB - 1\n        White & Grey - A - 2\n        Grey - Hold - 3\n*/\nlet msg1;\nlet msg2;\n\nnode.status({ text: msg.boilerDivV });\n\nswitch (msg.boilerDivV) {\n    case 0:     //HW ON (B)\n        msg1 = { topic: Sonoff1, payload: \"OFF\" } //White\n        msg2 = { topic: Sonoff2, payload: \"OFF\" } //Grey\n        global.set('boilerDivV', 0);\n        return [msg1, msg2]\n        break;\n    case 1:     //CH ON (AB)\n        msg1 = {topic: Sonoff1, payload: \"ON\"} //White\n        msg2 = { topic: Sonoff2, payload: \"OFF\" } //Grey\n        global.set('boilerDivV', 1);\n        return [msg1, msg2]\n        break;\n    case 2:     //HW OK (A)\n        msg1 = { topic: Sonoff1, payload: \"ON\" } //White\n        msg2 = { topic: Sonoff2, payload: \"ON\" } //Grey\n        global.set('boilerDivV')\n        return [msg1, msg2]\n        break;\n    case 3:     //HOLD (~~)\n        msg1 = { topic: Sonoff1, payload: \"OFF\" } //White\n        msg2 = { topic: Sonoff2, payload: \"ON\" } //Grey\n        global.set('boilerDivV', 3);\n        return [msg1, msg2]\n        break;  \n    case 4:     //Boiler OFF (B)\n        msg1 = { topic: Sonoff1, payload: \"OFF\" } //White\n        msg2 = { topic: Sonoff2, payload: \"OFF\" } //Grey\n        global.set('boilerDivV', 4);\n        return [msg1, msg2]\n        break;\n\n    default:     //OFF (B)\n        msg1 = { topic: Sonoff1, payload: \"OFF\" } //White\n        msg2 = { topic: Sonoff2, payload: \"OFF\" } //Grey\n        global.set('boilerDivV', 5);\n        return [msg1, msg2]\n        break;\n}","outputs":2,"timeout":0,"noerr":0,"initialize":"","finalize":"","libs":[],"x":1150,"y":960,"wires":[["ecfac1afe2bed870"],["06cd21e59f837b89"]]}]
2

Delete the break statements. Execution can never get to them.

3

Actually leaving the break statements in and moving the return down to the end would be a lot better I think.

4

Thank you @Colin.

The break was superfluous anyway, must have got mixed up with some other language coding!!

5

Tried that and get: [msg] Expected a 'break' statement before 'case'.
I agree, you would think that the return at the very end of the code should work, but doesn't, or at least, for the Linter.

6

It is ok for me

//let boilerDivV = global.get("boilerDivV");
let Sonoff1 = "cmnd/DivV/CH_on/POWER";
let Sonoff2 = "cmnd/DivV/HW_OK/POWER";

/*
        No Power - B - 0
        White - AB - 1
        White & Grey - A - 2
        Grey - Hold - 3
*/
let msg1;
let msg2;

node.status({ text: msg.boilerDivV });

switch (msg.boilerDivV) {
    case 0:     //HW ON (B)
        msg1 = { topic: Sonoff1, payload: "OFF" } //White
        msg2 = { topic: Sonoff2, payload: "OFF" } //Grey
        global.set('boilerDivV', 0);
        break;
    case 1:     //CH ON (AB)
        msg1 = {topic: Sonoff1, payload: "ON"} //White
        msg2 = { topic: Sonoff2, payload: "OFF" } //Grey
        global.set('boilerDivV', 1);
        break;
    case 2:     //HW OK (A)
        msg1 = { topic: Sonoff1, payload: "ON" } //White
        msg2 = { topic: Sonoff2, payload: "ON" } //Grey
        global.set('boilerDivV')
        break;
    case 3:     //HOLD (~~)
        msg1 = { topic: Sonoff1, payload: "OFF" } //White
        msg2 = { topic: Sonoff2, payload: "ON" } //Grey
        global.set('boilerDivV', 3);
        break;  
    case 4:     //Boiler OFF (B)
        msg1 = { topic: Sonoff1, payload: "OFF" } //White
        msg2 = { topic: Sonoff2, payload: "OFF" } //Grey
        global.set('boilerDivV', 4);
        break;

    default:     //OFF (B)
        msg1 = { topic: Sonoff1, payload: "OFF" } //White
        msg2 = { topic: Sonoff2, payload: "OFF" } //Grey
        global.set('boilerDivV', 5);
        break;
}
return [msg1, msg2]
7

A long time ago, I got into the habit of enclosing each cases code in braces which forces JavaScript to know that they are code blocks. Doesn't usually make much difference but occasionally will catch structural errors.

switch (msg.boilerDivV) {
    case 0:  {   //HW ON (B)
        msg1 = { topic: Sonoff1, payload: "OFF" } //White
        msg2 = { topic: Sonoff2, payload: "OFF" } //Grey
        global.set('boilerDivV', 0);
        break;
    }
    case 1: {    //CH ON (AB)
        msg1 = {topic: Sonoff1, payload: "ON"} //White
        msg2 = { topic: Sonoff2, payload: "OFF" } //Grey
        global.set('boilerDivV', 1);
        break;
    }
    case 2: {    //HW OK (A)
        msg1 = { topic: Sonoff1, payload: "ON" } //White
        msg2 = { topic: Sonoff2, payload: "ON" } //Grey
        global.set('boilerDivV')
        break;
    }
    case 3: {    //HOLD (~~)
        msg1 = { topic: Sonoff1, payload: "OFF" } //White
        msg2 = { topic: Sonoff2, payload: "ON" } //Grey
        global.set('boilerDivV', 3);
        break;
    }  
    case 4: {    //Boiler OFF (B)
        msg1 = { topic: Sonoff1, payload: "OFF" } //White
        msg2 = { topic: Sonoff2, payload: "OFF" } //Grey
        global.set('boilerDivV', 4);
        break;
    }
    default:  {   //OFF (B)
        msg1 = { topic: Sonoff1, payload: "OFF" } //White
        msg2 = { topic: Sonoff2, payload: "OFF" } //Grey
        global.set('boilerDivV', 5);
        break;
    }
}
8

Ah, I took the breaks out. I misunderstood what you were saying, and your solution looks good!!

@TotallyInformation I tend to put more Brackets in than most, it helps my brain, even with the counting up and down of left and right brackets. I think I will adopt this also.

9

break means jump to the end. They were redundant originally because there was a return before them so the execution never got to the break.

You can simplify your code further by using

let msg1 = {topic: Sonoff1}
let msg2 = {topic: Sonoff2}

node.status({ text: msg.boilerDivV });

switch (msg.boilerDivV) {
    case 0:     //HW ON (B)
        msg1.payload = "OFF"  //White
        msg2.payload: "OFF" //Grey
        global.set('boilerDivV', 0);
        break;

Also the definitions of Sonoff1 and 2 should be const. Then the interpreter is free to make optimisations based on that knowledge, and it will stop you accidentally reassigning a value to it.

10

Yes, I do that too.

11

I will update to this (I have several Nodes with similar function). Nice to see how my code can be improved upon, thank you!

12

Shouldn't this rather be

    msg2.payload = "OFF" //Grey

? :thinking:

13

Yes, previous post corrected

14

Next 'project' with NR is to change let to const as I come across them. These were written a while ago before const was the new let in the NR Forum.(Where I get most of my JS training from!) :smiley:

15

Do you mean var to let or const? This is something that the Node-RED core codebase also needs to do!

Though moving let to const where the variable shouldn't be updated is also sensible. Also noting that objects and arrays can be const if all you want to do is change an entry rather than replace the whole thing.

16

So, if a variable is defined at the beginning of a Function Node, and it does not change in the Function Node, it can be a constno matter what it is?

17

Yes, though strictly, I suppose you should call that a CONSTANT. :wink:

const one = {}
const two = 42
one.something = 'hello' // This is fine
one = {something: 'hello'} // This will fail
two = 24 // this also fails
18

Ah, I thought they meant CONSTANT as in a Number, not a letter/word/anything containing Alphabetic characters

19

Nope, it means that the named variable is a "constant" - sort-of. Actually means that you cannot completely replace the content of the named variable. That you can change the properties of a const object or array is one of the many strange parts of JavaScript.

20

...and so reigneth confusion again!!:rofl: