6

Noticed a weird thing, SOQL inside a for loop is a bad practice but in a while loop isn't?

Force.com security scanner has not detected a SOQL inside a while loop and also note that our paid application is already in AppExchange for past few years. So I am not sure why security is allowing this.

This is a concern because if same functionality is implemented in recursive function, it might complain.

Code snippet to traverse from child to root parent:

List<String> descriptionList = new List<String>();
while(accountId != null) {
    String parentAccount = null;

    for(Account account:[select Name, parentId from Account where id = :accountId]) {
        parentAccount = account.parentId;
        descriptionList.add(account.Name);
    }
    accountId = parentAccount;
}

I can do some workaround for to resolve the issue but curious to know on why Salesforce allows it.

  • 1
    Yeah, a query inside a while loop is bad, but is it a security issue? I haven't really used the security scanner myself, does it complain about queries inside of for loops? – Derek F Nov 17 '17 at 14:40
  • 4
    Its a (long) while since I used the scanner, but the last time I did I got the impression that the scanner was not doing a full parse and analysis of the code but some looser kind of pattern matching. So there were many false positives (that I looked at) and so potentially many false negatives (that I didn't look at). – Keith C Nov 17 '17 at 15:21
3

Neither PMD nor the Code Scanner consider this pattern to be a code violation for "SOQL inside a loop." That type of rule is more concerned with code that is not bulkified (e.g. using multiple queries when one would suffice) instead of this situation, where you're actually querying recursively, which has legitimate purposes, such as building an hierarchy of records.

| improve this answer | |
  • I've never found a need. You can often build hierarchies with a single query using a Map<Id, List<SObject>>. – Adrian Larson Nov 17 '17 at 18:04
  • @AdrianLarson Yes, it still requires multiple queries in a "loop", but it's not the same as the type of violation they're looking for (querying records singly instead of aggregating parent/child records into a single query). – sfdcfox Nov 17 '17 at 18:07
  • I was under assumption that salesforce doesn’t allow any app to be published on appexchage with any bad practices. Your explanation make sense. Thanks! – Raul Nov 18 '17 at 0:30
  • 1
    @RahulSharma They strongly recommend having code that performs well. There's nothing inherently "wrong" with the code in the question, although depending on its purpose, there may be a way to reduce the overall number of queries (e.g. by querying multiple levels at once, etc). – sfdcfox Nov 18 '17 at 1:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.