Salesforce's Apex Compiler Throwing Phantom Error

Question

I have code that compiles and executes fine in execute anonymous and when combined but seems to have a weird issue otherwise:

private static Boolean updateWorkOrder(Id workOrderId, String responseBody) {
    if(workOrderId == null || responseBody == null || !responseBody.contains('REQ -'))

    Integer pos = responseBody.indexOf('REQ -');
    String install = responseBody.substring(pos,responseBody.length());

    WorkOrder wo = new WorkOrder(Id = workOrderId, Snap_Install__c = install);
    Database.SaveResult sr = Database.update(wo,false);
    if(sr.isSuccess()) return true;
    else return false;
    return false;
}

I keep getting a "Variable Does Not Exist" error. With the code below, I get pos doesn't exist on the line below it (install = ...).

If I change the line to the below, I get an error that install does not exist on the wo line.

String install = responseBody.substring(responseBody.indexOf('REQ -'),responseBody.length())

And so on... The code runs just fine when I put it in execute anonymous.

I can get the class to compile as follows:

if(Database.update(new WorkOrder(Id = workOrderId, Snap_Install__c = responseBody.substring(responseBody.indexOf('REQ -'),responseBody.length())),false).isSuccess()) return true;
    else return false;
    return false;

But this is pretty ugly. Does anyone have any idea what is going on here?

Answer 1

I'd expect the problem to be leaving out optional brackets in your first if statement, so the compiler expects the next line (and only the next line) to be within that block. This means the pos variable isn't available on the next line, which is out of the scope of the if block.

I'm assuming you want the method to return immediately if your if statement is satisfied. Give this a shot:

private static Boolean updateWorkOrder(Id workOrderId, String responseBody) {
    if ( workOrderId == null || responseBody == null || !responseBody.contains('REQ -') ) {
        return false;
    }

    Integer pos = responseBody.indexOf('REQ -');
    String install = responseBody.substring(pos,responseBody.length());
    WorkOrder wo = new WorkOrder(Id = workOrderId, Snap_Install__c = install);
    Database.SaveResult sr = Database.update(wo,false);
    return sr.isSuccess();
}

Answer 2

pos has a (block) scope of the first if statement, it's created and destroyed within the block. Without explicit declaration of the block via curly braces, only the first instruction that follows the if statement belongs to the block. Your code

if(workOrderId == null || responseBody == null || !responseBody.contains('REQ -'))

    Integer pos = responseBody.indexOf('REQ -');
    String install = responseBody.substring(pos,responseBody.length());

is equivalent to

if(workOrderId == null || responseBody == null || !responseBody.contains('REQ -')) {
    Integer pos = responseBody.indexOf('REQ -');
}

String install = responseBody.substring(pos,responseBody.length());

When you attempt to use pos in responseBody.substring(...), the compiler correctly complains because pos does not exist at this point. This is why using braces to denote blocks is the best practice - it helps avoid issues like this one and it improves maintainability by making the code more readable.