1

I want to use a list I create in the main method when the Save button is clicked on my VF Page for example:

public with sharing class AddAccountsController {

    Public Map<id,ElectricAccountNum__c> maptoAccts {get;set;}

    public AddAccounts Controller() {
        List<Opportunity> AccID = [Select AccountId, EDC__c  From Opportunity Where Id =:ApexPages.currentPage().getParameters().get('id')];

        for(Opportunity a: AccId){
                      Map<id,ElectricAccountNum__c> maptoAccts = new map <id,ElectricAccountNum__c>([SELECT Id FROM ElectricAccountNum__c Where Retail_Account__c =:a.AccountId and EDC__c =: a.EDC__c and ID  IN(SELECT Electric_Account__c FROM Electric_Account_Deals__c where Opportunity__c =: ApexPages.currentPage().getParameters().get('id'))  ]);
        }
        //do other things
    }

    public PageReference save() {
        if(MaptoAccts.containskey(x)){
          //do other things
        }
    }
}

But when I system.debug in the PageReference save method I am returning a null mapping for MaptoAccts. Is this possible to send the list to this method?

1
  • 2
    In your for loop, you redefine maptoAccts map each time you pass in the loop ! – SF_user May 12 '17 at 15:06
2

The issue you're running into is called 'variable shadowing' (you also have a query inside of a for loop, which is a bad thing).

If you're unfamiliar with 'shadowing', here's a short example that explains it:

public class MyClass{
    public String myVar;
    public String myOtherVar;

    public MyClass(){
        // At this point, myVar and myOtherVar have not been initialized, and are thus null.
        // We can give one of them a simple value here
        myVar = 'abc';
    }

    public void myMethod(){
        // Because we're in a class method, we have access to class variables
        system.debug(myVar);        // This will display 'abc'
        system.debug(myOtherVar);   // This will display null
    }

    public void myOtherMethod(){
        // The line below these comments redeclares myOtherVar
        // By doing this, we say that 'myOtherVar' has become'shadowed'.
        // Because of scoping rules, when you reference 'myOtherVar' in this method, you'll
        //   get the value that is stored in the 'myOtherVar' that was declared in
        //   this method instead of the class-level 'myOtherVar'
        String myOtherVar = '123';

        system.debug(myVar);        // This will display 'abc'
        system.debug(myOtherVar);   // This will display '123'

        // You can still access the class-level variable, the one that has been
        //   'shadowed', by using the 'this' keyword.
        // Assigning a value to the myOtherVar that was defined in the scope of this
        //   method does not set nor overwrite the value of the 'shadowed' class-level variable
        system.debug(this.myOtherVar);    // This will display null
    }

There are two different ways to fix this issue in your code (don't re-declare the variable, or use this), but it's generally a good idea to avoid shadowing variables where possible (because it's easy to get confused as to which variable you're referring to).

My code ends up being pretty much exactly the same as the code offered in Eric's answer

public AddAccountsController() {
    List<Opportunity> AccID = [Select AccountId, EDC__c  From Opportunity Where Id =:ApexPages.currentPage().getParameters().get('id')];

    for(Opportunity a: AccId){
        // This shadows the class-level mapToAccts variable
        //Map<id,ElectricAccountNum__c> maptoAccts = new map <id,ElectricAccountNum__c>([SELECT Id FROM ElectricAccountNum__c Where Retail_Account__c =:a.AccountId and EDC__c =: a.EDC__c and ID  IN(SELECT Electric_Account__c FROM Electric_Account_Deals__c where Opportunity__c =: ApexPages.currentPage().getParameters().get('id'))  ]);

        // By not specifying the type of the variable (i.e. just using the variable and 
        //   assigning a value to it), we avoid the re-declaration
        //   (and thus we avoid shadowing the class-level variable as well)
        // Note that a query inside of a loop is still less than ideal
        maptoAccts = new map <id,ElectricAccountNum__c>([SELECT Id FROM ElectricAccountNum__c Where Retail_Account__c =:a.AccountId and EDC__c =: a.EDC__c and ID  IN(SELECT Electric_Account__c FROM Electric_Account_Deals__c where Opportunity__c =: ApexPages.currentPage().getParameters().get('id'))  ]);
    }
    //do other things
}

Doing this will ensure that the map you access in your save() method will contain the results from your query in your constructor. If your query in your constructor doesn't return any results, you should see an empty list (as opposed to null).

Removing your query from the loop is getting into the territory where making a new question is appropriate, but some quick advice on that:

  • It's harder to pull queries out of loops if your WHERE clause contains more than one filter that depends on data that you could aggregate into a List or Set. WHERE field IN :set1 AND field2 IN :set2 gives you the cartesian product instead of the conjunction
  • You could use a formula field to make a pseudo composite key (just a string concatenation of your two fields), and iterate over the result of your first query to build a collection of composite keys to use in your second query, but this might lead to query selectivity issues
  • You could just deal with getting the cartesian product by iterating over the results of your second query, and only keeping the results that simultaneously match your two filters for a single source record. This should keep a query selective, but incurs a CPU time cost (as final processing needs to be done in Apex instead of through SOQL)
4
  • Oh man, I have some to learn, just getting into this. I have taken your suggestions (I actually was already moving the query outside of my for loop because I have seen that is bad) but my class is now executing as expected. Thanks for your help – AverageJoeVBA May 12 '17 at 16:08
  • The query in the loop will only ever run once since the initial query can only return 0 or 1 records (querying against a single ID), unless I'm missing something. – IllusiveBrian May 12 '17 at 17:04
  • @IllusiveBrian You have a point. Though with that in mind, adhering to "loop = no queries or DML zone" is still a good habit. – Derek F May 12 '17 at 17:13
  • @DerekF Yeah, since this isn't codereview.se I'm refraining from explaining all the reasons using a loop this way is a bad idea, but making it difficult to tell whether you are handing one or many records is at the top of the list. – IllusiveBrian May 12 '17 at 17:23
1

I do not see where you are actually executing the controller method and you are using a local variable for the map.

These changes should get you going (not the anti-pattern of a query in a loop has not been addressed)

public AddAccounts Controller() {
    List<Opportunity> AccID = [Select AccountId, EDC__c  From Opportunity Where Id =:ApexPages.currentPage().getParameters().get('id')];
    maptoAccts = new map <id,ElectricAccountNum__c>(); //Instantiate class property
    for(Opportunity a: AccId){
         //Put all results into the map
         maptoAccts.putAll(new map <id,ElectricAccountNum__c>([SELECT Id FROM ElectricAccountNum__c Where Retail_Account__c =:a.AccountId and EDC__c =: a.EDC__c and ID  IN(SELECT Electric_Account__c FROM Electric_Account_Deals__c where Opportunity__c =: ApexPages.currentPage().getParameters().get('id'))  ]);
    }
    //do other things
}

Not sure of your data model but seems to me you could be using relationship subqueries to get at the data you need rather than putting the query in a loop....

2
  • I get results though if I system debug within the top controller, it is just when I get to the PageReference the system debug returns null. – AverageJoeVBA May 12 '17 at 15:53
  • Sorry it was a subtle difference I see what you mean now! – AverageJoeVBA May 12 '17 at 16:07
0

here is what I ended up with:

public AddAccts() {
 string AccID;
 string EDCs;
 for(Opportunity curr:[Select AccountId, EDC__c From Opportunity where id=: ApexPages.currentPage().getParameters().get('id')]){
            AccID= curr.AccountId;
            EDCs= curr.EDC__c;

 }
 maptoAccts = new map <id,ElectricAccountNum__c>([SELECT Id FROM ElectricAccountNum__c Where Retail_Account__c =:AccID and EDC__c=: EDCs and ID  IN(SELECT Electric_Account__c FROM Electric_Account_Deals__c where Opportunity__c =: ApexPages.currentPage().getParameters().get('id'))  ]);
 System.debug(maptoaccts);
}
public PageReference save() {
 System.debug(maptoaccts);
}

Both debugs are displaying the same information, so on to my next steps. Thanks for your help.

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.